Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c) - Mailing list pgsql-hackers

From Álvaro Herrera
Subject Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)
Date
Msg-id 202603091236.kw5fttkbzvrl@alvherre.pgsql
Whole thread Raw
In response to Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)
Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)
Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)
List pgsql-hackers
On 2026-Mar-09, Michael Paquier wrote:

> On Mon, Mar 09, 2026 at 11:21:35AM +0800, yangyz wrote:
> > I think it should be modified.
> > 
> > Move createPQExpBuffer inside the conditional block to match its destroy counterpart.
> > This improves code clarity and satisfies static analyzers, even though the actual memory
> > leak is minimal in practice.
> 
> destroyPQExpBuffer() is called for each tuple from pg_database except
> if dealing with "template{0,1}" or "postgres".  It means that we would
> just leak a few bytes for these three cases.  I agree that the
> variable declaration can be placed better, but it's really not worth
> bothering in this context.

True, but at the same time it looks as if this routine is wastefully
written -- I mean, why spend time with a stringinfo here at all?  We
could write this in much simpler form, as in the attached, which is even
three lines shorter.  In fact, before 763aaa06f034, this is exactly how
this routine was written, and I don't see why it was changed this way.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Just treat us the way you want to be treated + some extra allowance
 for ignorance."                                    (Michael Brusser)

Attachment

pgsql-hackers by date:

Previous
From: Amul Sul
Date:
Subject: Re: pg_waldump: support decoding of WAL inside tarfile
Next
From: Ranier Vilela
Date:
Subject: Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)