Re: LDAP controls in response
| From: | Jani Taskinen | Date: | Wed, 09 Nov 2005 08:03:54 +0000 |
| Subject: | Re: LDAP controls in response | ||
| References: | 1 2 3 4 5 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-19932@lists.php.net to get a copy of this message | ||
On Wed, 9 Nov 2005, Pierangelo Masarati wrote:
On Wed, 2005-11-09 at 07:05 +0200, Jani Taskinen wrote: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).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(),
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.
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.
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.
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.
--Jani