Re: Fix for bug in ldapServiceLookup in libpq - Mailing list pgsql-hackers

From Albe Laurenz
Subject Re: Fix for bug in ldapServiceLookup in libpq
Date
Msg-id D960CB61B694CF459DCFB4B0128514C20668FBB6@exadv11.host.magwien.gv.at
Whole thread Raw
In response to Re: Fix for bug in ldapServiceLookup in libpq  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Uninitialized SSL values? (was: Fix for bug in ldapServiceLookup in libpq)  ("Albe Laurenz" <laurenz.albe@wien.gv.at>)
Re: Fix for bug in ldapServiceLookup in libpq  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Tom Lane wrote:
>> I have found a small but annoying bug in libpq where
>> connection parameters are resolved via LDAP.

> Hmm ... that's a bug all right, but why have the null-termination
> inside the loop at all?  Seems like it should look like
>
>    for (p = result, i = 0; values[i] != NULL; ++i)
>    {
>        strncpy(p, values[i]->bv_val, values[i]->bv_len);
>        p += values[i]->bv_len;
>        *(p++) = '\n';
>    }
>    *p = '\0';

Yes, that's better.

> ... btw, shouldn't this function free the "result" string when it's
done
> with it?  AFAICS that string is not returned to the caller, it's just
> being leaked.

Oops, yes it should.

> (I'll refrain from asking why it's creating the string in the first
> place rather than parsing ldap_get_values_len's output as-is ...)

So that I can close the LDAP connection as soon as feasible, but of
course
that's not absolutely necessary.

I have attached a new version of the patch that should address all known
problems.

Yours,
Laurenz Albe

Attachment

pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: Tables cannot have INSTEAD OF triggers
Next
From: Kohei Kaigai
Date:
Subject: Re: [v9.2] Leaky view and RLS