Re: LDAP: bugfix and deprecated OpenLDAP API

On Tue, 2013-09-24 at 15:07 +0000, Albe Laurenz wrote:
> --- 3511,3534 ----
>   	}
>   
>   	/*
> ! 	 * Perform an explicit anonymous bind.
> ! 	 * This is not necessary in principle, but we want to set a timeout
> ! 	 * of PGLDAP_TIMEOUT seconds and return 2 if the connection fails.
> ! 	 * Unfortunately there is no standard conforming way to do that.
>   	 */

This comment has become a bit confusing.  What exactly is nonstandard?
Setting a timeout, or returning 2?  The code below actually returns 3.

> + #ifdef HAVE_LIBLDAP
> + 	/* in OpenLDAP, use the LDAP_OPT_NETWORK_TIMEOUT option */

We don't use HAVE_LIBLDAP anywhere else to mean OpenLDAP.  Existing
LDAP-related code uses #ifdef WIN32.

> + #else

There should be a comment here indicating what this #else belongs to
(#else /* HAVE_LIBLDAP */, or whatever we end up using).

> + 	/* the nonstandard ldap_connect function performs an anonymous bind */
> + 	if (ldap_connect(ld, &time) != LDAP_SUCCESS)
> + 	{
> + 		/* error or timeout in ldap_connect */
> + 		free(url);
> + 		ldap_unbind(ld);
> + 		return 2;
> + 	}
> + #endif

here too

Bonus: Write a commit message for your patch.  (Consider using git
format-patch.)