Re: Small psql memory fix

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

In response to

Responses

Browse pgsql-hackers by date

  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