revision: fix leaking display notes

We never free the display notes options embedded into `struct revision`.
Implement a new function `release_display_notes()` that we can call in
`release_revisions()` to fix this.

There is another gotcha here though: we play some games with the string
list used to track extra notes refs, where we sometimes set the bit that
indicates that strings should be strdup'd and sometimes unset it. This
dance is done to avoid a copy of an already-allocated string when we
call `enable_ref_display_notes()`. But this dance is rather pointless as
we can instead call `string_list_append_nodup()` to transfer ownership
of the allocated string to the list.

Refactor the code to do so and drop the `strdup_strings` dance.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/notes.c b/notes.c
index 53ca25c814738d2ec772f9d16193e9fc2424b3b7..6a157e34ce610379e9c7cdf466a44083c186629f 100644 (file)
--- a/notes.c
+++ b/notes.c
@@ -1060,6 +1060,12 @@ void init_display_notes(struct display_notes_opt *opt)
 {
        memset(opt, 0, sizeof(*opt));
        opt->use_default_notes = -1;
+       string_list_init_dup(&opt->extra_notes_refs);
+}
+
+void release_display_notes(struct display_notes_opt *opt)
+{
+       string_list_clear(&opt->extra_notes_refs, 0);
 }
 
 void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes)
@@ -1073,19 +1079,15 @@ void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes,
        struct strbuf buf = STRBUF_INIT;
        strbuf_addstr(&buf, ref);
        expand_notes_ref(&buf);
-       string_list_append(&opt->extra_notes_refs,
-                       strbuf_detach(&buf, NULL));
+       string_list_append_nodup(&opt->extra_notes_refs,
+                                strbuf_detach(&buf, NULL));
        *show_notes = 1;
 }
 
 void disable_display_notes(struct display_notes_opt *opt, int *show_notes)
 {
        opt->use_default_notes = -1;
-       /* we have been strdup'ing ourselves, so trick
-        * string_list into free()ing strings */
-       opt->extra_notes_refs.strdup_strings = 1;
        string_list_clear(&opt->extra_notes_refs, 0);
-       opt->extra_notes_refs.strdup_strings = 0;
        *show_notes = 0;
 }
 

diff --git a/notes.h b/notes.h
index 064fd7143aad33424714de671f08238bdd463190..235216944bcb4b5f01a4d626df704551fcd4d333 100644 (file)
--- a/notes.h
+++ b/notes.h
@@ -275,6 +275,11 @@ struct display_notes_opt {
  */
 void init_display_notes(struct display_notes_opt *opt);
 
+/*
+ * Release resources acquired by the display_notes_opt.
+ */
+void release_display_notes(struct display_notes_opt *opt);
+
 /*
  * This family of functions enables or disables the display of notes. In
  * particular, 'enable_default_display_notes' will display the default notes,

diff --git a/revision.c b/revision.c
index af95502d925783b5f7e8c34cbb55e6a83e4974d9..75e71bcaea8db39d18ef94031bd0c14f2dd02c53 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -3168,6 +3168,7 @@ void release_revisions(struct rev_info *revs)
 {
        free_commit_list(revs->commits);
        free_commit_list(revs->ancestry_path_bottoms);
+       release_display_notes(&revs->notes_opt);
        object_array_clear(&revs->pending);
        object_array_clear(&revs->boundary_commits);
        release_revisions_cmdline(&revs->cmdline);

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index cf23c06c098756abec968d689bcbf3e58541901b..536bd11ff4769f40c50392fb9e684d06b1e6ba83 100755 (executable)
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -5,6 +5,7 @@
 
 test_description='Test commit notes'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 write_script fake_editor <<\EOF