Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:19933
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <34721.131.175.154.56.1131525375.squirrel@131.175.154.56>
In-Reply-To: <Pine.LNX.4.61.0511090957290.23759@arfg.argcubovn.sv>
References: <1131485315.3327.15.camel@ando> 
	<Pine.LNX.4.61.0511090035400.17812@arfg.argcubovn.sv> 
	<1131492177.3327.25.camel@ando> 
	<Pine.LNX.4.61.0511090702110.21476@arfg.argcubovn.sv>
	<1131521067.20182.2.camel@ando>
	<Pine.LNX.4.61.0511090957290.23759@arfg.argcubovn.sv>
Date: Wed, 9 Nov 2005 09:36:15 +0100 (CET)
To: "Jani Taskinen" <sniper@iki.fi>
Cc: venaas@php.net, internals@lists.php.net
Reply-To: ando@sys-net.it
User-Agent: SquirrelMail/1.4.3a-1
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-8859-1
Content-Transfer-Encoding: 8bit
Importance: Normal
Subject: Re: [PHP-DEV] LDAP controls in response
From: ando@sys-net.it ("Pierangelo Masarati")


> 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:pierangelo.masarati@sys-net.it



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:    pierangelo.masarati@sys-net.it
------------------------------------------