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>

diff --git a/builtin/repack.c b/builtin/repack.c
index 62cfa50c50f893406803cbbedef711f35167cb14..8bb875532b4db6e3284b42c8e77bbafa114fee9b 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -732,14 +732,23 @@ static void midx_included_packs(struct string_list *include,
                                struct pack_geometry *geometry)
 {
        struct string_list_item *item;
+       struct strbuf buf = STRBUF_INIT;
+
+       for_each_string_list_item(item, &existing->kept_packs) {
+               strbuf_reset(&buf);
+               strbuf_addf(&buf, "%s.idx", item->string);
+               string_list_insert(include, buf.buf);
+       }
+
+       for_each_string_list_item(item, names) {
+               strbuf_reset(&buf);
+               strbuf_addf(&buf, "pack-%s.idx", item->string);
+               string_list_insert(include, buf.buf);
+       }
 
-       for_each_string_list_item(item, &existing->kept_packs)
-               string_list_insert(include, xstrfmt("%s.idx", item->string));
-       for_each_string_list_item(item, names)
-               string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
        if (geometry->split_factor) {
-               struct strbuf buf = STRBUF_INIT;
                uint32_t i;
+
                for (i = geometry->split; i < geometry->pack_nr; i++) {
                        struct packed_git *p = geometry->pack[i];
 
@@ -754,17 +763,21 @@ static void midx_included_packs(struct string_list *include,
                        if (!p->pack_local)
                                continue;
 
+                       strbuf_reset(&buf);
                        strbuf_addstr(&buf, pack_basename(p));
                        strbuf_strip_suffix(&buf, ".pack");
                        strbuf_addstr(&buf, ".idx");
 
-                       string_list_insert(include, strbuf_detach(&buf, NULL));
+                       string_list_insert(include, buf.buf);
                }
        } else {
                for_each_string_list_item(item, &existing->non_kept_packs) {
                        if (pack_is_marked_for_deletion(item))
                                continue;
-                       string_list_insert(include, xstrfmt("%s.idx", item->string));
+
+                       strbuf_reset(&buf);
+                       strbuf_addf(&buf, "%s.idx", item->string);
+                       string_list_insert(include, buf.buf);
                }
        }
 
@@ -784,8 +797,13 @@ static void midx_included_packs(struct string_list *include,
                 */
                if (pack_is_marked_for_deletion(item))
                        continue;
-               string_list_insert(include, xstrfmt("%s.idx", item->string));
+
+               strbuf_reset(&buf);
+               strbuf_addf(&buf, "%s.idx", item->string);
+               string_list_insert(include, buf.buf);
        }
+
+       strbuf_release(&buf);
 }
 
 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)
                mark_packs_for_deletion(&existing, &names);
 
        if (write_midx) {
-               struct string_list include = STRING_LIST_INIT_NODUP;
+               struct string_list include = STRING_LIST_INIT_DUP;
                midx_included_packs(&include, &existing, &names, &geometry);
 
                ret = write_midx_included_packs(&include, &geometry, &names,