builtin/repack: fix leaks when computing packs to repack
When writing an MIDX in git-repack(1) we first collect all the pack names that we want to add to it in a string list. This list is marked as `NODUP`, which indicates that it will neither duplicate nor own strings added to it. In `write_midx_included_packs()` we then `insert()` strings via `xstrdup()` or `strbuf_detach()`, but the resulting strings will not be owned by anything and thus leak. Fix this issue by marking the list as `DUP` and using a local buffer to compute the pack names. This leak is hit in t5319, but plugging it is not sufficient to make the whole test suite pass. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Aug 22, 2024 at 11:17 UTC
bda97cb1199919a192cc821a9e03b810b7d96f8f
1 file changed
+27
-9
builtin/repack.c
+27
-9
@@ -732,14 +732,23 @@ static void midx_included_packs(struct string_list *include,
732
struct pack_geometry *geometry)
733
{
734
struct string_list_item *item;
735
+ struct strbuf buf = STRBUF_INIT;
736
+
737
+ for_each_string_list_item(item, &existing->kept_packs) {
738
+ strbuf_reset(&buf);
739
+ strbuf_addf(&buf, "%s.idx", item->string);
740
+ string_list_insert(include, buf.buf);
741
+ }
742
+
743
+ for_each_string_list_item(item, names) {
744
+ strbuf_reset(&buf);
745
+ strbuf_addf(&buf, "pack-%s.idx", item->string);
746
+ string_list_insert(include, buf.buf);
747
+ }
748
736
- for_each_string_list_item(item, &existing->kept_packs)
737
- string_list_insert(include, xstrfmt("%s.idx", item->string));
738
- for_each_string_list_item(item, names)
739
- string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
749
if (geometry->split_factor) {
741
- struct strbuf buf = STRBUF_INIT;
750
uint32_t i;
751
+
752
for (i = geometry->split; i < geometry->pack_nr; i++) {
753
struct packed_git *p = geometry->pack[i];
754
@@ -754,17 +763,21 @@ static void midx_included_packs(struct string_list *include,
763
if (!p->pack_local)
764
continue;
765
766
+ strbuf_reset(&buf);
767
strbuf_addstr(&buf, pack_basename(p));
768
strbuf_strip_suffix(&buf, ".pack");
769
strbuf_addstr(&buf, ".idx");
770
761
- string_list_insert(include, strbuf_detach(&buf, NULL));
771
+ string_list_insert(include, buf.buf);
772
}
773
} else {
774
for_each_string_list_item(item, &existing->non_kept_packs) {
775
if (pack_is_marked_for_deletion(item))
776
continue;
767
- string_list_insert(include, xstrfmt("%s.idx", item->string));
777
+
778
+ strbuf_reset(&buf);
779
+ strbuf_addf(&buf, "%s.idx", item->string);
780
+ string_list_insert(include, buf.buf);
781
}
782
}
783
@@ -784,8 +797,13 @@ static void midx_included_packs(struct string_list *include,
797
*/
798
if (pack_is_marked_for_deletion(item))
799
continue;
787
- string_list_insert(include, xstrfmt("%s.idx", item->string));
800
+
801
+ strbuf_reset(&buf);
802
+ strbuf_addf(&buf, "%s.idx", item->string);
803
+ string_list_insert(include, buf.buf);
804
}
805
+
806
+ strbuf_release(&buf);
807
}
808
809
static int write_midx_included_packs(struct string_list *include,
@@ -1476,7 +1494,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1494
mark_packs_for_deletion(&existing, &names);
1495
1496
if (write_midx) {
1479
- struct string_list include = STRING_LIST_INIT_NODUP;
1497
+ struct string_list include = STRING_LIST_INIT_DUP;
1498
midx_included_packs(&include, &existing, &names, &geometry);
1499
1500
ret = write_midx_included_packs(&include, &geometry, &names,