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.


If this function is reused in the future or used in long-lived processes, it 
might become a problem.

Regards,
Yuanzhuo Yang




         Original
         
       
From: Chao Li <[email protected]&gt;
Date: 2026-03-09 08:04
To: Ranier Vilela <[email protected]&gt;
Cc: Pg Hackers <[email protected]&gt;
Subject: Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c)





&gt;&nbsp;On&nbsp;Mar&nbsp;9,&nbsp;2026,&nbsp;at&nbsp;07:05,&nbsp;Ranier&nbsp;Vilela&nbsp;<[email protected]&gt;&nbsp;wrote:
&gt;&nbsp;
&gt;&nbsp;Hi.
&gt;&nbsp;
&gt;&nbsp;Per&nbsp;Coverity.
&gt;&nbsp;
&gt;&nbsp;Coverity&nbsp;complains&nbsp;about&nbsp;one&nbsp;resource&nbsp;leak&nbsp;in&nbsp;the&nbsp;function
&gt;&nbsp;*drops_DBs*.
&gt;&nbsp;
&gt;&nbsp;CID&nbsp;1645454:&nbsp;(#1&nbsp;of&nbsp;1):&nbsp;Resource&nbsp;leak&nbsp;(RESOURCE_LEAK)&nbsp;
&gt;&nbsp;19.&nbsp;leaked_storage:&nbsp;Variable&nbsp;delQry&nbsp;going&nbsp;out&nbsp;of&nbsp;scope&nbsp;leaks&nbsp;the&nbsp;storage&nbsp;it&nbsp;points&nbsp;to.
&gt;&nbsp;
&gt;&nbsp;Fix&nbsp;by&nbsp;avoiding&nbsp;creating&nbsp;the&nbsp;buffer&nbsp;unnecessarily.
&gt;&nbsp;
&gt;&nbsp;Trivial&nbsp;patch&nbsp;attached.
&gt;&nbsp;
&gt;&nbsp;best&nbsp;regards,
&gt;&nbsp;Ranier&nbsp;Vilela
&gt;&nbsp;<avoid-resource-leak-pg_dumpall.patch&gt;

I&nbsp;confirmed&nbsp;this&nbsp;is&nbsp;a&nbsp;leak,&nbsp;but&nbsp;only&nbsp;leaks&nbsp;3&nbsp;tuples,&nbsp;not&nbsp;much&nbsp;memory&nbsp;leaked.&nbsp;Given&nbsp;pg_dump&nbsp;call&nbsp;is&nbsp;not&nbsp;a&nbsp;long-live&nbsp;frontend&nbsp;process,&nbsp;such&nbsp;leak&nbsp;won’t&nbsp;hurt&nbsp;much.

Instead&nbsp;of&nbsp;claiming&nbsp;a&nbsp;memory&nbsp;leak,&nbsp;I&nbsp;would&nbsp;tend&nbsp;to&nbsp;claim&nbsp;a&nbsp;logic&nbsp;mistake.&nbsp;Because&nbsp;createPQExpBuffer&nbsp;is&nbsp;before&nbsp;the&nbsp;“if”&nbsp;clause,&nbsp;but&nbsp;the&nbsp;corresponding&nbsp;destroyPQExpBuffer&nbsp;is&nbsp;placed&nbsp;inside&nbsp;the&nbsp;“if”&nbsp;clause,&nbsp;they&nbsp;are&nbsp;logically&nbsp;mismatch.

Best&nbsp;regards,
--
Chao&nbsp;Li&nbsp;(Evan)
HighGo&nbsp;Software&nbsp;Co.,&nbsp;Ltd.
https://www.highgo.com/

Reply via email to