Subject: Re: [PATCH v3 15/16] repack: introduce `--write-midx=incremental`

From: Jeff King <[email-masked]>
Date: Wed, 13 May 2026 19:08:25 -0400

On Wed, Apr 29, 2026 at 08:13:49PM -0400, Taylor Blau wrote:

> +void clear_incremental_midx_files(struct repository *r,
> +				  const struct strvec *keep_hashes)
> +{
> +	struct strbuf chain = STRBUF_INIT;
> +
> +	get_midx_chain_filename(r->objects->sources, &chain);
> +
> +	if (r->objects) {
> +		struct odb_source *source = r->objects->sources;
> +		for (source = r->objects->sources; source; source = source->next) {
> +			struct odb_source_files *files = odb_source_files_downcast(source);
> +			if (files->packed->midx)
> +				close_midx(files->packed->midx);
> +			files->packed->midx = NULL;
> +		}
> +	}

Can r->objects be NULL here? If so, then the get_midx_chain_filename()
call will segfault, since we look at r->objects->sources. If not, then
this conditional is tautological and can be dropped.

Looking at the two callers, the one in repack_remove_redundant_pack()
unconditionally looks at repo->objects->sources itself, so we know it is
not NULL there.

The other is in write_midx_incremental(), but the flow there is less
clear.

I'd guess it is probably the case that it is always non-NULL so there is
no bug, but dropping the conditional would make the code more clear.

-Peff