From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Small psql memory fix |
Date: | 2014-02-15 04:52:42 |
Message-ID: | 1250.1392439962@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Bruce Momjian <bruce(at)momjian(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2014-02-15 04:56:53 | Re: Small psql memory fix |
Previous Message | Bruce Momjian | 2014-02-15 04:35:08 | Re: Small psql memory fix |