builtin/fast-export: plug leaking tag names

When resolving revisions in `get_tags_and_duplicates()`, we only partially manage the lifetime of `full_name`. In fact, managing its lifetime properly is almost impossible because we put direct pointers to that variable into multiple lists without duplicating the string. The consequence is that these strings will ultimately leak. Refactor the code to make the lists we put those names into duplicate the memory. This allows us to properly free the string as required and thus plugs the memory leak. While this requires us to allocate more data overall, it shouldn't be all that bad given that the number of allocations corresponds with the number of command line parameters, which typically aren't all that many. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 14, 2024 at 08:52 UTC a0b82622cbb31de66d7f5f0b1e39f349edaeb009
2 files changed +13 -5
builtin/fast-export.c
+12 -5
@@ -42,8 +42,8 @@ static int full_tree;
42 static int reference_excluded_commits;
43 static int show_original_ids;
44 static int mark_tags;
45 -static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
46 -static struct string_list tag_refs = STRING_LIST_INIT_NODUP;
45 +static struct string_list extra_refs = STRING_LIST_INIT_DUP;
46 +static struct string_list tag_refs = STRING_LIST_INIT_DUP;
47 static struct refspec refspecs = REFSPEC_INIT_FETCH;
48 static int anonymize;
49 static struct hashmap anonymized_seeds;
@@ -901,7 +901,7 @@ static void handle_tag(const char *name, struct tag *tag)
901 free(buf);
902 }
903
904 -static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)
904 +static struct commit *get_commit(struct rev_cmdline_entry *e, const char *full_name)
905 {
906 switch (e->item->type) {
907 case OBJ_COMMIT:
@@ -932,14 +932,16 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
932 struct rev_cmdline_entry *e = info->rev + i;
933 struct object_id oid;
934 struct commit *commit;
935 - char *full_name;
935 + char *full_name = NULL;
936
937 if (e->flags & UNINTERESTING)
938 continue;
939
940 if (repo_dwim_ref(the_repository, e->name, strlen(e->name),
941 - &oid, &full_name, 0) != 1)
941 + &oid, &full_name, 0) != 1) {
942 + free(full_name);
943 continue;
944 + }
945
946 if (refspecs.nr) {
947 char *private;
@@ -955,6 +957,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
957 warning("%s: Unexpected object of type %s, skipping.",
958 e->name,
959 type_name(e->item->type));
960 + free(full_name);
961 continue;
962 }
963
@@ -963,10 +966,12 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
966 break;
967 case OBJ_BLOB:
968 export_blob(&commit->object.oid);
969 + free(full_name);
970 continue;
971 default: /* OBJ_TAG (nested tags) is already handled */
972 warning("Tag points to object of unexpected type %s, skipping.",
973 type_name(commit->object.type));
974 + free(full_name);
975 continue;
976 }
977
@@ -979,6 +984,8 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
984
985 if (!*revision_sources_at(&revision_sources, commit))
986 *revision_sources_at(&revision_sources, commit) = full_name;
987 + else
988 + free(full_name);
989 }
990
991 string_list_sort(&extra_refs);
t/t9351-fast-export-anonymize.sh
+1
@@ -4,6 +4,7 @@ test_description='basic tests for fast-export --anonymize'
4 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
5 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
6
7 +TEST_PASSES_SANITIZE_LEAK=true
8 . ./test-lib.sh
9
10 test_expect_success 'setup simple repo' '