Peter Eisentraut 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.
I have improved the comment.
>> + #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.
Changed.
> > + #else > > There should be a comment here indicating what this #else belongs to > (#else /* HAVE_LIBLDAP */, or whatever we end up using).
Changed.
>> + /* 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
Changed.
> Bonus: Write a commit message for your patch. (Consider using git > format-patch.)
Suggested commit message is included in the patch.
Sorry about the huge delay in trying to get around to this one.
For the sake of the archives - I looked at the fact that the windows codepath always returns 2, whereas the unix codepath separates at least one case out as a return 3. I can't see a pattern in the windows return codes that would make it possible to do the same though, without explicitly listing different error codes mapping to each of them - so I don't think it's worth doing that.
Thus - applied, and backpatched all the way. Thanks!
(And I just realized I forgot to credit reviewers in the commit message. My apologies - I blame confusion after havnig to re-setup my windows build environments all over again..)