Skip to content

Commit 187b623

Browse files
pks-tgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1ca57be commit 187b623

File tree

3 files changed

+6
-5
lines changed

3 files changed

+6
-5
lines changed

builtin/notes.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ static int merge_commit(struct notes_merge_options *o)
807807
{
808808
struct strbuf msg = STRBUF_INIT;
809809
struct object_id oid, parent_oid;
810-
struct notes_tree *t;
810+
struct notes_tree t = {0};
811811
struct commit *partial;
812812
struct pretty_print_context pretty_ctx;
813813
void *local_ref_to_free;
@@ -830,16 +830,15 @@ static int merge_commit(struct notes_merge_options *o)
830830
else
831831
oidclr(&parent_oid, the_repository->hash_algo);
832832

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);
835834

836835
o->local_ref = local_ref_to_free =
837836
refs_resolve_refdup(get_main_ref_store(the_repository),
838837
"NOTES_MERGE_REF", 0, &oid, NULL);
839838
if (!o->local_ref)
840839
die(_("failed to resolve NOTES_MERGE_REF"));
841840

842-
if (notes_merge_commit(o, t, partial, &oid))
841+
if (notes_merge_commit(o, &t, partial, &oid))
843842
die(_("failed to finalize notes merge"));
844843

845844
/* Reuse existing commit message in reflog message */
@@ -853,7 +852,7 @@ static int merge_commit(struct notes_merge_options *o)
853852
is_null_oid(&parent_oid) ? NULL : &parent_oid,
854853
0, UPDATE_REFS_DIE_ON_ERR);
855854

856-
free_notes(t);
855+
free_notes(&t);
857856
strbuf_release(&msg);
858857
ret = merge_abort(o);
859858
free(local_ref_to_free);

t/t3310-notes-merge-manual-resolve.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
test_description='Test notes merging with manual conflict resolution'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
# Set up a notes merge scenario with different kinds of conflicts

t/t3311-notes-merge-fanout.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
test_description='Test notes merging at various fanout levels'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
verify_notes () {

0 commit comments

Comments
 (0)