Avoid resource leak (src/test/modules/test_binaryheap/test_binaryheap.c)
Hi.
Per Coverity.
Coverity reports this resource leak in test_binaryheap module.
I think that is right.
Trivial patch attached.
best regards,
Ranier Vilela
Attachments:
avoid-resource-leak-test_binaryheap.patchapplication/octet-stream; name=avoid-resource-leak-test_binaryheap.patchDownload
diff --git a/src/test/modules/test_binaryheap/test_binaryheap.c b/src/test/modules/test_binaryheap/test_binaryheap.c
index 583dae1da3..ca0781ded6 100644
--- a/src/test/modules/test_binaryheap/test_binaryheap.c
+++ b/src/test/modules/test_binaryheap/test_binaryheap.c
@@ -135,6 +135,7 @@ test_basic(int size)
if (!binaryheap_empty(heap))
elog(ERROR, "heap not empty after removing all nodes");
+ binaryheap_free(heap);
}
/*
@@ -154,6 +155,7 @@ test_build(int size)
binaryheap_build(heap);
verify_heap_property(heap);
+ binaryheap_free(heap);
}
/*
@@ -181,6 +183,7 @@ test_remove_node(int size)
if (binaryheap_size(heap) != size - remove_count)
elog(ERROR, "wrong size after removing nodes");
+ binaryheap_free(heap);
}
/*
@@ -211,6 +214,7 @@ test_replace_first(int size)
*/
binaryheap_replace_first(heap, Int32GetDatum(size + 1));
verify_heap_property(heap);
+ binaryheap_free(heap);
}
/*
@@ -230,6 +234,7 @@ test_duplicates(int size)
if (DatumGetInt32(binaryheap_remove_first(heap)) != dup)
elog(ERROR, "unexpected value in heap with duplicates");
}
+ binaryheap_free(heap);
}
/*
@@ -247,6 +252,7 @@ test_reset(int size)
if (!binaryheap_empty(heap))
elog(ERROR, "heap not empty after resetting");
+ binaryheap_free(heap);
}
/*
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
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