Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 7, 2021, 04:15 UTC
Message-ID: <YV50cEN6hdvNOrpE@nand.local>
In-Reply-To: <87wnmph73b.fsf@evledraar.gmail.com>

On Thu, Oct 07, 2021 at 04:57:10AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 5 quoted lines

> @@ -1927,11 +1929,9 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
>  	}
>
>  	result = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);
> -	m = NULL;

This was the only hunk that gave me a little bit of pause. But it is the right thing to be doing.

When this code was originally written, write_midx_internal took a pointer to a struct multi_pack_index, and sometimes called close_midx() on that pointer. Calling close_midx() on the same pointer is UB, since close_midx() frees the provided pointer.

In other words, when this code was originally written, the 'm' variable in midx_repack() could become stale.

But since f57a739691 (midx: avoid opening multiple MIDXs when writing, 2021-09-01), we don't pass a pointer into write_midx_internal(), only the path to an object directory. So we can always call close_midx() here.

Thanks, Taylor