Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)

Indeed, createPQExpBuffer is not needed. v2 is much better. Looks good to me.


         Original
         
       
From: Ranier Vilela <ranier.vf@gmail.com&gt;
Date: 2026-03-09 20:54
To: Álvaro Herrera <alvherre@kurilemu.de&gt;
Cc: Michael Paquier <michael@paquier.xyz&gt;, yangyz <1197620467@qq.com&gt;, Chao Li <li.evan.chao@gmail.com&gt;, Pg Hackers <pgsql-hackers@postgresql.org&gt;
Subject: Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)







Em seg., 9 de mar. de 2026 às 09:40, Álvaro Herrera <alvherre@kurilemu.de&gt; escreveu:
On 2026-Mar-09, Michael Paquier wrote:
 
 &gt; On Mon, Mar 09, 2026 at 11:21:35AM +0800, yangyz wrote:
 &gt; &gt; I think it should be modified.
 &gt; &gt;
 &gt; &gt; Move createPQExpBuffer inside the conditional block to match its destroy counterpart.
 &gt; &gt; This improves code clarity and satisfies static analyzers, even though the actual memory
 &gt; &gt; leak is minimal in practice.
 &gt;
 &gt; destroyPQExpBuffer() is called for each tuple from pg_database except
 &gt; if dealing with "template{0,1}" or "postgres".&nbsp; It means that we would
 &gt; just leak a few bytes for these three cases.&nbsp; I agree that the
 &gt; variable declaration can be placed better, but it's really not worth
 &gt; 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?&nbsp; We
 could write this in much simpler form, as in the attached, which is even
 three lines shorter.&nbsp; In fact, before 763aaa06f034, this is exactly how
 this routine was written, and I don't see why it was changed this way.
+1


LGTM.


best regards,
Ranier Vilela&nbsp;

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