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>

diff --git a/builtin/notes.c b/builtin/notes.c
index d9c356e3543fecadfdbd29fd2fe27062b1a2baa9..81cbaeec6b64866bf1829fbc920938710789cea9 100644 (file)
--- 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 597df5ebc0a582018d30c018e734d1154c364ed6..04866b89bed3146fe9506c748c0b824ef70b12b3 100755 (executable)
--- 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 5b675417e9bbf9d21cac561473c90622b08def5a..ce4144db0f24c7bd9ccbbe30575118b565bb4486 100755 (executable)
--- 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 () {