Hi Rainer. 

> Am 12.08.2015 um 13:00 schrieb Rainer Jung <rainer.j...@kippdata.de>:
> 
> Hi Côme,
> 
>> Am 11.08.2015 um 16:58 schrieb Côme BERNIGAUD:
>>> On 2015-08-11 00:36, Rainer Jung wrote:
>>> The current problems should be mostly around the above four compiler
>>> warnings. I can test any patches you want me to test.
>> 
>> Can you test including the attached file in ext/ldap/ldap.c, and not
>> defining HAVE_3ARG_SETREBINDPROC and LDAP_CONTROL_PAGEDRESULTS ?
> 
> thanks for the patch proposal. There's a few problems with it and I will come 
> back to you with an alternative proposal soon. For the moment:
> 
> 
> - the definition of ldap_control_find in terms of at the top of ldap.c 
> currently is protected by
> 
> #if !defined(HAVE_LDAP_CONTROL_FIND)
> 
> It should be
> 
> #if defined(LDAP_CONTROL_PAGEDRESULTS) && !defined(HAVE_LDAP_CONTROL_FIND)
> 
> That's not related to your patch.
> 
> 
> - in
> 
> void ldap_memvfree( void **value )
> {
>  ldap_value_free(value);
> }
> 
> 
> it should probably be
> 
> ldap_value_free((char **)value);
> 
> otherwise we get compiler warnings.
> 
> 
> - instead of ldap_open() we can use ldap_init() (recommended).
> 
> 
> - using ldap_open() or ldap_init() with a url does not work, we have to use 
> host and port.
> 
> 
> - the use of ldap_sasl_bind_s() in PHP_FUNCTION(ldap_bind) does not work, 
> without setting "LDAP_OPT_PROTOCOL_VERSION" to LDAP_VERSION3 using 
> ldap_set_option(). Maybe it works for OpenLDAP, but Netscape and Solaris 
> document it must be set and indeed return LDAP_NOT_SUPPORTED immediately 
> otherwise. But since in PHP_FUNCTION(ldap_bind) no SASL functionality is 
> actually being used, I propose to use ldap_simple_bind_s() instead of 
> ldap_sasl_bind_s(). It is not deprecated and using it actually simplifies the 
> code a bit.
> 
> 
> As I said, I'll come up with a patch proposal soon.
> 
> The patch should also help for other LDAP implementations than OpenLDAP or 
> Solaris.
> 
The main question here is whether we want PHP to support other implementations. 
The documentation clearly states that OpenLDAP shall be used and even Oracle 
states on an official site (I don't actually have the URL at hand but it has 
been in an email to this thread) that PHPs LDAP extension has to be built 
against OpenLDAP. 

Come has done a great job to clean up the LDAP code to start implementing long 
awaited features and I'm not really sure whether we should give that up for 
maintaining compatibility with some edge-case usages. So I'm very interested in 
your patch to see how we can rech both goals!

In the meantime you can still use the "old" ext/ldap for the newer PHP-branch 
as it should compile without an issue. The "newer" ext/ldap just removed stuff 
;)

Cheers

Andreas
> Regards,
> 
> Rainer

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to