Mithun Cy <mithun.cy@enterprisedb.com> writes:
> On Thu, Oct 13, 2016 at 1:40 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>> I am attaching that to the next CF.
> One thing which you might need to reconsider is removal of memory leak
> comments. There is still a leak if there is an error while encoding in
> db_encoding_strdup.
> Unless you want to catch those error with an TRY();....CATCH(); and then
> free the mem.
I could have lived with leaving the leak there, but really this wasn't
fixing the worst problem with the code: if it did throw an error out of
the middle of that sequence, it would leave the process setlocale'd to
some other locale than we want. That could lead to unwanted behavior
in printf and other functions. And this isn't all that unlikely: an
encoding conversion failure is definitely possible if you have a locale
selected that's not compatible with the database encoding.
I whacked the patch around enough so that we didn't do anything except
libc calls between setting and restoring the locale. At that point it
was just a matter of adding a TRY block to be able to say that we
didn't leak any strdup'd strings, so I figured "might as well".
Pushed with those changes.
regards, tom lane