> On Wed, 9 Nov 2005, Pierangelo Masarati wrote:
>
>>
>> On Wed, 2005-11-09 at 07:05 +0200, Jani Taskinen wrote:
>>> The "control-patch" looks okay. But the "warning-fix-patch" really
>>> doesn't.
>>> You're actually changing some functions behaviour with some of
>>> those
>>> changes. Maybe you didn't notice but there's ldap_get_values() and
>>> ldap_get_values_len() separately. And there's ldap_bind() and
>>> ldap_sasl_bind(),
>>
>> What I'm doing is driven by the fact that the ldap_*value*() family, and
>> the ldap_bind*() are being deprecated (that is: they're in the library,
>> but they're not declared in ldap.h any more, and they are implemented as
>> wrappers to the replacements I'm using, at the cost of extra execution
>> time and resources, plus decent compiler warnings).
>
> Perhaps with OpenLDAP, but what about the other implementations with
> which this extension works just fine? You didn't wrap all those
> changes
> inside the #ifdef's.
Right, sorry. And note that this is true only for OpenLDAP 2.3 (which is
considered stable by the OpenLDAP project but maybe not by distributors
yet...). I'm pretty confident the reworking I put in place is conformant
with most APIs, but, as I said, I likely have no chance to test it
shortly, so I wouldn't stress too much on it.
>
>> ldap_*value*_len() is the safe implementation of ldap_*value*(), because
>> it can also handle non-null terminated values; this doesn't inhibit the
>> handling null-terminated ones. Moreover, PHP needs the length of the
>> string anyway, so... Note that this would allow PHP to use only
>> ldap_get_values() for both, making the API more simple and robust (I
>> vaguely recall the issue that brought to implementing
>> ldap_get_values_len() in PHP, I think it was somewhere in PHP 3 and the
>> patch took a while to get there).
>
> I agree it's better this way, but (even as unlikely it is) changing
> this
> MIGHT break someone's script.
Well, GIGO :) If one uses ldap_get_values_len() with strings (i.e.
null-terminated arrays) the result is fine and consistent; the BER
contains the info about the value length so there's not even the
performance penalty of a strlen(). If one was relying on using
ldap_get_values() for non-null terminated strings ...
>
> Anyway, it might be the time to cleanup the API of this extension
> and make it simpler. It's pretty confusing as it is.
> The returned arrays are also weird with their "count" fields and
> such.
I added them for consistency with the rest of the data returned by PHP's
ldap_get_{entries,attributes,values}(); I'm fine if they get removed, we
also save the counter...
Should I do the cleanup? Should I simply remove the "warning" stuff and
the "count" stuff in the arrays?
>
>> Anyway, none of these is an issue to me, so I'm fine with the controls
>> patch. A final comment: apart from some fuzziness complaints, both
>> patches apply also to PHP 4 without changes. This means we can get into
>> production immediately (well, after some testing... :)
>
> This is a new feature and new features are no longer added in PHP 4.
> We only add new stuff in PHP >= 5.1.
I was speaking for our own packages; usually I don't expect anything to be
backported so far, it's fair enough to see contributed features in the
mainstream at some point.
Thanks for your time. p.
--
Pierangelo Masarati
mailto:[EMAIL PROTECTED]
Ing. Pierangelo Masarati
Responsabile Open Solution
SysNet s.n.c.
Via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
------------------------------------------
Office: +39.02.23998309
Mobile: +39.333.4963172
Email: [EMAIL PROTECTED]
------------------------------------------
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
