Bruce Momjian <bruce@momjian.us> writes:
> On Fri, Feb 14, 2014 at 11:28:49PM -0500, Tom Lane wrote:
>> The first and third hunks look to me like they introduce memory
>> leaks, not fix them. The second hunk is probably safe enough,
> The first and third just move the free into blocks where we have already
> tested the value is not null. It just more clearly matches the
> surrounding code. Do you see something different?
[ looks closer... ] OK, they're not actually broken, but I question
whether they're improvements. The existing coding is clear enough
that the result of psql_scan_slash_option will be freed. What you're
doing is conflating that requirement with some other processing.
>> although I'm not sure the case can actually occur --- gset should
>> free the prefix before any new backslash command can be executed.
> Oh, interesting idea. Updated patch attached.
I don't think that's an improvement at all. My point was that I
didn't think gset_prefix would ever be set at entry to this code,
because the setting should never survive for more than one backslash
command execution cycle.
If you want to add probably-useless code to free it, go ahead, but
this isn't a clearer way to do that than your previous version.
regards, tom lane
