Michael Paquier <mich...@paquier.xyz> writes: > On Sat, Sep 03, 2022 at 05:00:30PM -0700, Zhihong Yu wrote: >> Please see the attached patch which frees the search_message in the above >> case.
> Yep, nice catch, I am reading the same thing as you do. I can see > that we already do that after a failing ldap_search_st() call in > fe-connect.c for libpq. Hence, similarly, we'd better call > ldap_msgfree() on search_message when it is not NULL after a search > failure, no? The patch you are proposing does not do that. 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. 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. 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. regards, tom lane