@matheusbsbs / matheus-bagliano / commits / 9748537437

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>

Patrick Steinhardt committed Jun 11, 2024 at 11:19 UTC 97485374377fa62fdd36f4b707e2fcd8f1a7c6c3

4 files changed +15 -6

notes.c

+8 -6

@@ -1060,6 +1060,12 @@ void init_display_notes(struct display_notes_opt *opt)

1060 {

1061 memset(opt, 0, sizeof(*opt));

1062 opt->use_default_notes = -1;

1063 + string_list_init_dup(&opt->extra_notes_refs);

1064 +}

1065 +

1066 +void release_display_notes(struct display_notes_opt *opt)

1067 +{

1068 + string_list_clear(&opt->extra_notes_refs, 0);

1069 }

1070

1071 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,

1079 struct strbuf buf = STRBUF_INIT;

1080 strbuf_addstr(&buf, ref);

1081 expand_notes_ref(&buf);

1076 - string_list_append(&opt->extra_notes_refs,

1077 - strbuf_detach(&buf, NULL));

1082 + string_list_append_nodup(&opt->extra_notes_refs,

1083 + strbuf_detach(&buf, NULL));

1084 *show_notes = 1;

1085 }

1086

1087 void disable_display_notes(struct display_notes_opt *opt, int *show_notes)

1088 {

1089 opt->use_default_notes = -1;

1084 - /* we have been strdup'ing ourselves, so trick

1085 - * string_list into free()ing strings */

1086 - opt->extra_notes_refs.strdup_strings = 1;

1090 string_list_clear(&opt->extra_notes_refs, 0);

1088 - opt->extra_notes_refs.strdup_strings = 0;

1091 *show_notes = 0;

1092 }

1093

notes.h

@@ -275,6 +275,11 @@ struct display_notes_opt {

275 */

276 void init_display_notes(struct display_notes_opt *opt);

277

278 +/*

279 + * Release resources acquired by the display_notes_opt.

280 + */

281 +void release_display_notes(struct display_notes_opt *opt);

282 +

283 /*

284 * This family of functions enables or disables the display of notes. In

285 * particular, 'enable_default_display_notes' will display the default notes,

revision.c

@@ -3168,6 +3168,7 @@ void release_revisions(struct rev_info *revs)

3168 {

3169 free_commit_list(revs->commits);

3170 free_commit_list(revs->ancestry_path_bottoms);

3171 + release_display_notes(&revs->notes_opt);

3172 object_array_clear(&revs->pending);

3173 object_array_clear(&revs->boundary_commits);

3174 release_revisions_cmdline(&revs->cmdline);

t/t3301-notes.sh

@@ -5,6 +5,7 @@

6 test_description='Test commit notes'

8 +TEST_PASSES_SANITIZE_LEAK=true

9 . ./test-lib.sh

11 write_script fake_editor <<\EOF