On Fri, Feb 14, 2014 at 11:52:42PM -0500, Tom Lane wrote:
> 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.
If you think this code isn't helping, I will just discard it.
-- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB
http://enterprisedb.com
+ Everyone has their own god. +