midx: use `strvec` for `keep_hashes`

The `keep_hashes` array in `write_midx_internal()` accumulates the
checksums of MIDX files that should be retained when pruning stale
entries from the MIDX chain. For similar reasons as in a previous
commit, rewrite this using a strvec, requiring us to pass one fewer
parameter.

Unlike the aforementioned previous commit, use a `strvec` instead of a
`string_list`, which provides a more ergonomic interface to adjust the
values at a particular index. The ordering is important here, as this
value is used to determine the contents of the resulting
`multi-pack-index-chain` file when writing with "--incremental".

Since the previous commit already builds the array in forward order, the
conversion is straightforward: replace indexed assignments with
`strvec_push()`, drop the pre-counting and `CALLOC_ARRAY()`, and
simplify cleanup via `strvec_clear()`.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/midx-write.c b/midx-write.c
index 55c778a97cbe59b7e4b7501364f451edc6f97c90..5d9409a9741eee34e268415aae44cff3fe02bee6 100644 (file)
--- a/midx-write.c
+++ b/midx-write.c
@@ -29,8 +29,7 @@ extern void clear_midx_files_ext(struct odb_source *source, const char *ext,
                                 const char *keep_hash);
 extern void clear_incremental_midx_files_ext(struct odb_source *source,
                                             const char *ext,
-                                            const char **keep_hashes,
-                                            uint32_t hashes_nr);
+                                            const struct strvec *keep_hashes);
 extern int cmp_idx_or_pack_name(const char *idx_or_pack_name,
                                const char *idx_name);
 
@@ -1109,8 +1108,7 @@ done:
 }
 
 static void clear_midx_files(struct odb_source *source,
-                            const char **hashes, uint32_t hashes_nr,
-                            unsigned incremental)
+                            const struct strvec *hashes, unsigned incremental)
 {
        /*
         * if incremental:
@@ -1124,13 +1122,15 @@ static void clear_midx_files(struct odb_source *source,
         */
        struct strbuf buf = STRBUF_INIT;
        const char *exts[] = { MIDX_EXT_BITMAP, MIDX_EXT_REV, MIDX_EXT_MIDX };
-       uint32_t i, j;
+       uint32_t i;
 
        for (i = 0; i < ARRAY_SIZE(exts); i++) {
-               clear_incremental_midx_files_ext(source, exts[i],
-                                                hashes, hashes_nr);
-               for (j = 0; j < hashes_nr; j++)
-                       clear_midx_files_ext(source, exts[i], hashes[j]);
+               clear_incremental_midx_files_ext(source, exts[i], hashes);
+               if (hashes) {
+                       for (size_t j = 0; j < hashes->nr; j++)
+                               clear_midx_files_ext(source, exts[i],
+                                                    hashes->v[j]);
+               }
        }
 
        if (incremental)
@@ -1267,8 +1267,7 @@ static int write_midx_internal(struct write_midx_opts *opts)
        int pack_name_concat_len = 0;
        int dropped_packs = 0;
        int result = -1;
-       const char **keep_hashes = NULL;
-       size_t keep_hashes_nr = 0;
+       struct strvec keep_hashes = STRVEC_INIT;
        struct chunkfile *cf;
 
        trace2_region_enter("midx", "write_midx_internal", r);
@@ -1708,32 +1707,12 @@ static int write_midx_internal(struct write_midx_opts *opts)
        if (ctx.num_multi_pack_indexes_before == UINT32_MAX)
                die(_("too many multi-pack-indexes"));
 
-       if (ctx.compact) {
-               struct multi_pack_index *m;
-
-               /*
-                * Keep all MIDX layers excluding those in the range [from, to].
-                */
-               for (m = ctx.base_midx; m; m = m->base_midx)
-                       keep_hashes_nr++;
-               for (m = ctx.m;
-                    m && midx_hashcmp(m, ctx.compact_to, r->hash_algo);
-                    m = m->base_midx)
-                       keep_hashes_nr++;
-
-               keep_hashes_nr++; /* include the compacted layer */
-       } else {
-               keep_hashes_nr = ctx.num_multi_pack_indexes_before + 1;
-       }
-       CALLOC_ARRAY(keep_hashes, keep_hashes_nr);
-
        if (ctx.incremental) {
                FILE *chainf = fdopen_lock_file(&lk, "w");
                struct strbuf final_midx_name = STRBUF_INIT;
                struct multi_pack_index *m = ctx.base_midx;
                struct multi_pack_index **layers = NULL;
                size_t layers_nr = 0, layers_alloc = 0;
-               size_t j = 0;
 
                if (!chainf) {
                        error_errno(_("unable to open multi-pack-index chain file"));
@@ -1761,12 +1740,12 @@ static int write_midx_internal(struct write_midx_opts *opts)
                                layers[layers_nr++] = mp;
                        }
                        while (layers_nr)
-                               keep_hashes[j++] =
-                                       xstrdup(midx_get_checksum_hex(layers[--layers_nr]));
+                               strvec_push(&keep_hashes,
+                                           midx_get_checksum_hex(layers[--layers_nr]));
 
-                       keep_hashes[j++] =
-                               xstrdup(hash_to_hex_algop(midx_hash,
-                                                         r->hash_algo));
+                       strvec_push(&keep_hashes,
+                                   hash_to_hex_algop(midx_hash,
+                                                    r->hash_algo));
 
                        for (mp = ctx.m;
                             mp && midx_hashcmp(mp, ctx.compact_to,
@@ -1776,31 +1755,29 @@ static int write_midx_internal(struct write_midx_opts *opts)
                                layers[layers_nr++] = mp;
                        }
                        while (layers_nr)
-                               keep_hashes[j++] =
-                                       xstrdup(midx_get_checksum_hex(layers[--layers_nr]));
+                               strvec_push(&keep_hashes,
+                                           midx_get_checksum_hex(layers[--layers_nr]));
                } else {
                        for (; m; m = m->base_midx) {
                                ALLOC_GROW(layers, layers_nr + 1, layers_alloc);
                                layers[layers_nr++] = m;
                        }
                        while (layers_nr)
-                               keep_hashes[j++] =
-                                       xstrdup(midx_get_checksum_hex(layers[--layers_nr]));
+                               strvec_push(&keep_hashes,
+                                           midx_get_checksum_hex(layers[--layers_nr]));
 
-                       keep_hashes[j++] =
-                               xstrdup(hash_to_hex_algop(midx_hash,
-                                                         r->hash_algo));
+                       strvec_push(&keep_hashes,
+                                   hash_to_hex_algop(midx_hash,
+                                                    r->hash_algo));
                }
 
-               ASSERT(j == keep_hashes_nr);
-
                free(layers);
 
-               for (uint32_t i = 0; i < j; i++)
-                       fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]);
+               for (size_t i = 0; i < keep_hashes.nr; i++)
+                       fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes.v[i]);
        } else {
-               keep_hashes[ctx.num_multi_pack_indexes_before] =
-                       xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo));
+               strvec_push(&keep_hashes,
+                           hash_to_hex_algop(midx_hash, r->hash_algo));
        }
 
        if (ctx.m || ctx.base_midx)
@@ -1809,8 +1786,7 @@ static int write_midx_internal(struct write_midx_opts *opts)
        if (commit_lock_file(&lk) < 0)
                die_errno(_("could not write multi-pack-index"));
 
-       clear_midx_files(opts->source, keep_hashes, keep_hashes_nr,
-                        ctx.incremental);
+       clear_midx_files(opts->source, &keep_hashes, ctx.incremental);
        result = 0;
 
 cleanup:
@@ -1826,11 +1802,7 @@ cleanup:
        free(ctx.entries);
        free(ctx.pack_perm);
        free(ctx.pack_order);
-       if (keep_hashes) {
-               for (uint32_t i = 0; i < keep_hashes_nr; i++)
-                       free((char *)keep_hashes[i]);
-               free(keep_hashes);
-       }
+       strvec_clear(&keep_hashes);
        strbuf_release(&midx_name);
        close_midx(midx_to_free);
 

diff --git a/midx.c b/midx.c
index f75e3c9fa6dcfea1e2fe0b6ef00b38fb153c9efd..bcb8c999015980543ab4103972250764383a8b82 100644 (file)
--- a/midx.c
+++ b/midx.c
@@ -12,6 +12,7 @@
 #include "chunk-format.h"
 #include "pack-bitmap.h"
 #include "pack-revindex.h"
+#include "strvec.h"
 
 #define MIDX_PACK_ERROR ((void *)(intptr_t)-1)
 
@@ -19,8 +20,7 @@ int midx_checksum_valid(struct multi_pack_index *m);
 void clear_midx_files_ext(struct odb_source *source, const char *ext,
                          const char *keep_hash);
 void clear_incremental_midx_files_ext(struct odb_source *source, const char *ext,
-                                     char **keep_hashes,
-                                     uint32_t hashes_nr);
+                                     const struct strvec *keep_hashes);
 int cmp_idx_or_pack_name(const char *idx_or_pack_name,
                         const char *idx_name);
 
@@ -799,22 +799,22 @@ void clear_midx_files_ext(struct odb_source *source, const char *ext,
 }
 
 void clear_incremental_midx_files_ext(struct odb_source *source, const char *ext,
-                                     char **keep_hashes,
-                                     uint32_t hashes_nr)
+                                     const struct strvec *keep_hashes)
 {
        struct clear_midx_data data = {
                .keep = STRSET_INIT,
                .ext = ext,
        };
        struct strbuf buf = STRBUF_INIT;
-       uint32_t i;
 
-       for (i = 0; i < hashes_nr; i++) {
-               strbuf_reset(&buf);
-               strbuf_addf(&buf, "multi-pack-index-%s.%s", keep_hashes[i],
-                           ext);
+       if (keep_hashes) {
+               for (size_t i = 0; i < keep_hashes->nr; i++) {
+                       strbuf_reset(&buf);
+                       strbuf_addf(&buf, "multi-pack-index-%s.%s",
+                                   keep_hashes->v[i], ext);
 
-               strset_add(&data.keep, buf.buf);
+                       strset_add(&data.keep, buf.buf);
+               }
        }
 
        for_each_file_in_pack_subdir(source->path, "multi-pack-index.d",