From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Zhihong Yu <zyu(at)yugabyte(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: freeing LDAPMessage in CheckLDAPAuth |
Date: | 2022-09-04 07:25:18 |
Message-ID: | YxRS3i6tfDpDjDRy@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Sep 04, 2022 at 01:52:10AM -0400, Tom Lane wrote:
> I can't get too excited about this. All of the error exit paths in
> backend authentication code will lead immediately to process exit, so
> the possibility of some memory being leaked really has no consequences
> worth worrying about. If we *were* worried about it, sprinkling a few
> more ldap_msgfree() calls into the existing code would hardly make it
> more bulletproof.
Even if this is not critical in the backend for this authentication
path, I'd like to think that it is still a good practice for future
code so as anything code-pasted around would get the call. So I see
no reason to not put smth on HEAD at least.
> There's lots of psprintf() and other
> Postgres-universe calls in that code that could potentially fail and
> force an elog exit without reaching ldap_msgfree. So if you wanted to
> make this completely clean you'd need to resort to doing the freeing
> in PG_CATCH blocks ... and I don't see any value in hacking it to that
> extent.
Agreed. I cannot get excited about going down to that in this case.
> What might be worth inspecting is the code paths in frontend libpq
> that call ldap_msgfree(), because on the client side we don't get to
> assume that an error will lead to immediate process exit. If we've
> missed any cleanups over there, that *would* be worth fixing.
FWIW, I have looked at the frontend while writing my previous message
and did not notice anything.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2022-09-04 07:38:33 | Re: pgsql: pg_collation_actual_version() -> pg_collation_current_version(). |
Previous Message | John Naylor | 2022-09-04 06:12:52 | Re: [RFC] building postgres with meson - v12 |