Re: [PATCH 02/16] midx: use `string_list` for retained MIDX files

[Jeff King <peff@peff.net>]

On Sun, Mar 29, 2026 at 05:40:55PM -0400, Taylor Blau wrote:

> @@ -767,15 +766,12 @@ static void clear_midx_file_ext(const char *full_path, size_t full_path_len UNUS
>  				const char *file_name, void *_data)
>  {
>  	struct clear_midx_data *data = _data;
> -	uint32_t i;
>  
>  	if (!(starts_with(file_name, "multi-pack-index-") &&
>  	      ends_with(file_name, data->ext)))
>  		return;
> -	for (i = 0; i < data->keep_nr; i++) {
> -		if (!strcmp(data->keep[i], file_name))
> -			return;
> -	}
> +	if (string_list_has_string(&data->keep, file_name))
> +		return;

We are using has_string() here, so &data->keep will need to be sorted
(just keeping it in mind as we read further).

>  	if (keep_hash) {
> -		ALLOC_ARRAY(data.keep, 1);
> +		struct strbuf buf = STRBUF_INIT;
> +		strbuf_addf(&buf, "multi-pack-index-%s.%s", keep_hash, ext);
>  
> -		data.keep[0] = xstrfmt("multi-pack-index-%s.%s", keep_hash, ext);
> -		data.keep_nr = 1;
> +		string_list_insert(&data.keep, buf.buf);

OK, we use insert() here. But there is no difference between insert and
append, since the list only has one element. It is sorted either way. ;)

And we do not have to worry about being accidentally-quadratic because
there is no loop.

> -	ALLOC_ARRAY(data.keep, hashes_nr);
>  	for (i = 0; i < hashes_nr; i++)
> -		data.keep[i] = xstrfmt("multi-pack-index-%s.%s", keep_hashes[i],
> -				       ext);
> -	data.keep_nr = hashes_nr;
> -	data.ext = ext;
> +		string_list_append(&data.keep,
> +				   xstrfmt("multi-pack-index-%s.%s",
> +					   keep_hashes[i], ext));
> +	string_list_sort(&data.keep);

OK, and here we really do have a list, but we do append+sort to avoid
being quadratic. Good.

Since AFAICT we only care about set inclusion, I probably would have
used a strset instead, but for the numbers we are talking about it is
unlikely to matter much either way.

So looking good.

-Peff