Re: LDAP: bugfix and deprecated OpenLDAP API

From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>
Cc: Abhijit Menon-Sen *EXTERN* <ams(at)2ndQuadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: LDAP: bugfix and deprecated OpenLDAP API
Date: 2013-10-17 02:01:30
Message-ID: 1381975290.19926.12.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2013-10-17 02:10:44 Re: libpgport vs libpgcommon
Previous Message Peter Eisentraut 2013-10-17 01:41:20 libpgport vs libpgcommon