From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Silence resource leaks alerts |
Date: | 2025-04-11 05:37:24 |
Message-ID: | 764090.1744349844@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Thu, Apr 10, 2025 at 03:10:02PM -0300, Ranier Vilela wrote:
>> While it is arguable that this is a false warning, there is a benefit in
>> moving the initialization of the string buffer, silencing the warnings that
>> are presented in this case.
>>
>> 1. pg_overexplain.c
>> 2. ruleutils.c
> These code paths are far from being critical and the two ones in
> ruleutils.c are older, even if it is a practice that had better be
> discouraged particularly as initStringInfo() can allocate some memory
> for nothing. So it could bloat the current memory context if these
> code paths are repeatedly taken.
I don't believe either module ever gets run in a long-lived memory
context. So I think the burden of proof to show that these leaks
are meaningful is on the proposer.
I'm totally not on board with the approach of "if Coverity says this
is a leak then we must fix it". By and large, it's more efficient
for us to leak small allocations and recover them at context reset or
delete than it is to pfree those allocations retail. Sure, if we're
talking about big allocations, or if there will be a lot of such
allocations during the lifetime of the context, we'd better do the
retail pfrees. Sadly, such criteria are outside Coverity's ken.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | YeXiu | 2025-04-11 05:47:59 | Re: Feature Recommendations for Logical Subscriptions |
Previous Message | Zhijie Hou (Fujitsu) | 2025-04-11 05:24:13 | Fix replica identity checks for MERGE command on published table. |