On Thu, Oct 07, 2021 at 04:57:10AM +0200, Ævar Arnfjörð Bjarmason wrote: > @@ -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
