From 046a8686a406ebff7ca01dd4a3fabf9a679e010f Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 19 May 2026 11:57:48 -0400 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- midx-write.c | 84 ++++++++++++++++++---------------------------------- midx.c | 20 ++++++------- 2 files changed, 38 insertions(+), 66 deletions(-) diff --git a/midx-write.c b/midx-write.c index 55c778a97c..5d9409a974 100644 --- 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 f75e3c9fa6..bcb8c99901 100644 --- 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", -- 2.47.3