Re: Passing a valid result to ldap_first_entry? (ITS#984)

[Pierangelo Masarati <pmasarati@bci.it>]

John Morrissey wrote:

> I write mod_ldap, an LDAP authentication module for ProFTPD
> (www.proftpd.net). I received a report from a user a few days ago; he had
> filed ITS #984 and Kurt responded that it's not proper to call
> ldap_first_entry() with a NULL second argument, a result which should be
> returned by the previous call to ldap_search*().
>
> Anyway, here's my code:
>
> LDAP *ld;
> LDAPMessage *result, *e;
>
> if (ldap_search_st(ld, prefix, ldap_search_scope, filter, ldap_attrs, 0,
>                    &ldap_querytimeout_tp, &result) == -1) {
>     /* Handle a failed search */
> }
>
> if ((e = ldap_first_entry(ld, result)) == NULL) {
>     /* Handle a failed ldap_first_entry() */
> }

Your code is basically correct; what Kurt meant in
http://www.openldap.org/its/index.cgi/Incoming?id=984;user=guest;selectid=984
is that you should not rely on ldap_first_entry to trap NULL res; you
should check it before you call ldap_first_entry. The point is that
synchronous searches do NOT return -1 in case of failure; they rather
return an error code OR LDAP_SUCCESS in case everything was fine.
Your code should read:


LDAPMessage *result = NULL, *e; /* you shouldn't rely
                                   on ldap_seach_s*
                                   clearing the value of
                                   res in case of failure
                                   ... */
int rc;
rc = ldap_search_st(ld, prefix, ldap_search_scope, filter, ldap_attrs, 0,
                    &ldap_querytimeout_tp, &result);
if ( rc != LDAP_SUCCESS /* "paranoid" || res == NULL */ ) {
    /* handle failure */
}
if ((e = ldap_first_entry(ld, result)) == NULL) {
    /* Handle a failed ldap_first_entry() */
}

Bye, Pierangelo.