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.

Yours,
Laurenz Albe

Attachment: ldap-bug-4.patch
Description: ldap-bug-4.patch

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to