Thread

1. Avoid resource leak (src/test/modules/test_binaryheap/test_binaryheap.c)

   Ranier Vilela <ranier.vf@gmail.com> — 2025-09-12T17:53:56Z

   Hi.
   
   Per Coverity.
   
   Coverity reports this resource leak in test_binaryheap module.
   I think that is right.
   
   Trivial patch attached.
   
   best regards,
   Ranier Vilela
   

2. Re: Avoid resource leak (src/test/modules/test_binaryheap/test_binaryheap.c)

   Robert Haas <robertmhaas@gmail.com> — 2025-09-12T18:57:09Z

   On Fri, Sep 12, 2025 at 1:54 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
   > Per Coverity.
   >
   > Coverity reports this resource leak in test_binaryheap module.
   > I think that is right.
   >
   > Trivial patch attached.
   
   If this were correct, we'd need to also free the memory in all the
   error paths. But of course, in both error and non-error paths, we rely
   on memory context cleanup to free memory for us, except in cases where
   there's some specific reason to believe that's not good enough. I
   doubt that there is any such reason in this case.
   
   See src/backend/utils/mmgr/README
   
   -- 
   Robert Haas
   EDB: http://www.enterprisedb.com
   
   
   
   

3. Re: Avoid resource leak (src/test/modules/test_binaryheap/test_binaryheap.c)

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-12T22:53:10Z

   Robert Haas <robertmhaas@gmail.com> writes:
   > On Fri, Sep 12, 2025 at 1:54 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
   >> Coverity reports this resource leak in test_binaryheap module.
   >> I think that is right.
   
   > If this were correct, we'd need to also free the memory in all the
   > error paths. But of course, in both error and non-error paths, we rely
   > on memory context cleanup to free memory for us, except in cases where
   > there's some specific reason to believe that's not good enough. I
   > doubt that there is any such reason in this case.
   
   I agree this isn't interesting from a resource-leak perspective.
   However, is it interesting from a test-coverage perspective?
   AFAICS, test_binaryheap doesn't presently exercise binaryheap_free,
   which seems a little sad for what's supposed to be a unit-test
   module.
   
   Of course, binaryheap_free is quite trivial and we do already
   have coverage of it elsewhere.  So I'm not super excited about
   the omission.
   
   			regards, tom lane