builtin/notes: fix leaking `struct notes_tree` when merging notes
We allocate a `struct notes_tree` in `merge_commit()` which we then initialize via `init_notes()`. It's not really necessary to allocate the structure though given that we never pass ownership to the caller. Furthermore, the allocation leads to a memory leak because despite its name, `free_notes()` doesn't free the `notes_tree` but only clears it. Fix this issue by converting the code to use an on-stack variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Aug 14, 2024 at 08:52 UTC
187b623eeff215b56d8e45ebab1de899e974383a
3 files changed
+6
-5
builtin/notes.c
+4
-5
@@ -807,7 +807,7 @@ static int merge_commit(struct notes_merge_options *o)
807
{
808
struct strbuf msg = STRBUF_INIT;
809
struct object_id oid, parent_oid;
810
- struct notes_tree *t;
810
+ struct notes_tree t = {0};
811
struct commit *partial;
812
struct pretty_print_context pretty_ctx;
813
void *local_ref_to_free;
@@ -830,8 +830,7 @@ static int merge_commit(struct notes_merge_options *o)
830
else
831
oidclr(&parent_oid, the_repository->hash_algo);
832
833
- CALLOC_ARRAY(t, 1);
834
- init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
833
+ init_notes(&t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
834
835
o->local_ref = local_ref_to_free =
836
refs_resolve_refdup(get_main_ref_store(the_repository),
@@ -839,7 +838,7 @@ static int merge_commit(struct notes_merge_options *o)
838
if (!o->local_ref)
839
die(_("failed to resolve NOTES_MERGE_REF"));
840
842
- if (notes_merge_commit(o, t, partial, &oid))
841
+ if (notes_merge_commit(o, &t, partial, &oid))
842
die(_("failed to finalize notes merge"));
843
844
/* Reuse existing commit message in reflog message */
@@ -853,7 +852,7 @@ static int merge_commit(struct notes_merge_options *o)
852
is_null_oid(&parent_oid) ? NULL : &parent_oid,
853
0, UPDATE_REFS_DIE_ON_ERR);
854
856
- free_notes(t);
855
+ free_notes(&t);
856
strbuf_release(&msg);
857
ret = merge_abort(o);
858
free(local_ref_to_free);
t/t3310-notes-merge-manual-resolve.sh
+1
@@ -5,6 +5,7 @@
5
6
test_description='Test notes merging with manual conflict resolution'
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
# Set up a notes merge scenario with different kinds of conflicts
t/t3311-notes-merge-fanout.sh
+1
@@ -5,6 +5,7 @@
5
6
test_description='Test notes merging at various fanout levels'
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
verify_notes () {