From: Patrick Steinhardt Date: Wed, 14 Aug 2024 06:52:20 +0000 (+0200) Subject: builtin/notes: fix leaking `struct notes_tree` when merging notes X-Git-Tag: v2.47.0-rc0~104^2~12 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=187b623eeff215b56d8e45ebab1de899e974383a;p=thirdparty%2Fgit.git 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 Signed-off-by: Junio C Hamano --- diff --git a/builtin/notes.c b/builtin/notes.c index d9c356e354..81cbaeec6b 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -807,7 +807,7 @@ static int merge_commit(struct notes_merge_options *o) { struct strbuf msg = STRBUF_INIT; struct object_id oid, parent_oid; - struct notes_tree *t; + struct notes_tree t = {0}; struct commit *partial; struct pretty_print_context pretty_ctx; void *local_ref_to_free; @@ -830,8 +830,7 @@ static int merge_commit(struct notes_merge_options *o) else oidclr(&parent_oid, the_repository->hash_algo); - CALLOC_ARRAY(t, 1); - init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0); + init_notes(&t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0); o->local_ref = local_ref_to_free = refs_resolve_refdup(get_main_ref_store(the_repository), @@ -839,7 +838,7 @@ static int merge_commit(struct notes_merge_options *o) if (!o->local_ref) die(_("failed to resolve NOTES_MERGE_REF")); - if (notes_merge_commit(o, t, partial, &oid)) + if (notes_merge_commit(o, &t, partial, &oid)) die(_("failed to finalize notes merge")); /* Reuse existing commit message in reflog message */ @@ -853,7 +852,7 @@ static int merge_commit(struct notes_merge_options *o) is_null_oid(&parent_oid) ? NULL : &parent_oid, 0, UPDATE_REFS_DIE_ON_ERR); - free_notes(t); + free_notes(&t); strbuf_release(&msg); ret = merge_abort(o); free(local_ref_to_free); diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh index 597df5ebc0..04866b89be 100755 --- a/t/t3310-notes-merge-manual-resolve.sh +++ b/t/t3310-notes-merge-manual-resolve.sh @@ -5,6 +5,7 @@ test_description='Test notes merging with manual conflict resolution' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # Set up a notes merge scenario with different kinds of conflicts diff --git a/t/t3311-notes-merge-fanout.sh b/t/t3311-notes-merge-fanout.sh index 5b675417e9..ce4144db0f 100755 --- a/t/t3311-notes-merge-fanout.sh +++ b/t/t3311-notes-merge-fanout.sh @@ -5,6 +5,7 @@ test_description='Test notes merging at various fanout levels' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh verify_notes () {