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.
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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
