[PATCH v2 5/6] string-list API users: don't tweak "strdup_strings" to free dupes

[Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>]

Change code added in [1] and [2] used in notes.c and bisect.c to
initialize a "struct string_list" as "DUP" and append with "nodup",
rather than doing it the other way around.

Settings up a "struct string_list" as "NODUP" and then manually
freeing its items by flipping the "strdup_strings" breaks the
encapsulation of the "struct string_list". It's also both more verbose
than the alternative, and not as safe. If we miss one of the codepaths
that appends to the list we'll end up freeing a constant string.

It's better to declare it as "dup", and then when we insert into the
list declare that the particular string we're inserting should be
owned by the "struct string_list" with string_list_append_nodup(). The
worst case with that API use is that we'll miss a caller, end up
double-dup-ing the string, and have a memory leak as a result.

1. 92e0d42539a (revision.c: make --no-notes reset --notes list,
   2011-03-29)
2. fb71a329964 (bisect--helper: `bisect_clean_state` shell function in
   C, 2017-09-29)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
 bisect.c | 7 +++----
 notes.c  | 8 ++------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/bisect.c b/bisect.c
index b63669cc9d7..8412feb1f69 100644
--- a/bisect.c
+++ b/bisect.c
@@ -1159,7 +1159,7 @@ static int mark_for_removal(const char *refname, const struct object_id *oid,
 {
 	struct string_list *refs = cb_data;
 	char *ref = xstrfmt("refs/bisect%s", refname);
-	string_list_append(refs, ref);
+	string_list_append_nodup(refs, ref);
 	return 0;
 }
 
@@ -1168,11 +1168,10 @@ int bisect_clean_state(void)
 	int result = 0;
 
 	/* There may be some refs packed during bisection */
-	struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
+	struct string_list refs_for_removal = STRING_LIST_INIT_DUP;
 	for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
-	string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
+	string_list_append_nodup(&refs_for_removal, xstrdup("BISECT_HEAD"));
 	result = delete_refs("bisect: remove", &refs_for_removal, REF_NO_DEREF);
-	refs_for_removal.strdup_strings = 1;
 	string_list_clear(&refs_for_removal, 0);
 	unlink_or_warn(git_path_bisect_expected_rev());
 	unlink_or_warn(git_path_bisect_ancestors_ok());
diff --git a/notes.c b/notes.c
index 7452e71cc8d..acc35b580b6 100644
--- a/notes.c
+++ b/notes.c
@@ -1064,19 +1064,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;
 }
 
-- 
2.37.1.1095.g64a1e8362fd