Re: [PATCH 3/8] midx: stop using linked list when closing MIDX

On Thu, Jul 10, 2025 at 07:22:58PM -0400, Taylor Blau wrote:

On Wed, Jul 09, 2025 at 09:54:51AM +0200, Patrick Steinhardt wrote:

quoted

diff --git a/midx.c b/midx.c
index a91231bfcdf..416b3e8b54f 100644
--- a/midx.c
+++ b/midx.c

@@ -835,11 +834,13 @@ void clear_midx_file(struct repository *r)

 	get_midx_filename(r->hash_algo, &midx, r->objects->sources->path);

-	if (r->objects && r->objects->multi_pack_index) {
-		close_midx(r->objects->multi_pack_index);
-		r->objects->multi_pack_index = NULL;
-		for (struct odb_source *source = r->objects->sources; source; source = source->next)
+	if (r->objects) {
+		for (struct odb_source *source = r->objects->sources; source; source = source->next) {
+			if (source->multi_pack_index)
+				close_midx(source->multi_pack_index);
 			source->multi_pack_index = NULL;

...and then this line would NULL the now-free()'d memory out.

But instead we are directly iterating through the sources and both
closing and NULL-ing out their respective MIDXs (if any).

As an aside: I know we do the C99-style for loop with declarations in
many places, but in this instance it seems to have produced an awfully
long line. I wonder if in this instance it would be better to write:

    struct odb_source *source;
    for (source = r->objects->sources; source; source = source->next) {
        /* ... */
    }

That's still a little lengthy, but it's fewer than 80 characters ;-).

Fair, can adapt.

quoted

+		}
+		r->objects->multi_pack_index = NULL;

Presumably this pointer will go away at some point in the future as
well?

Yup, exactly, it's removed at the end of this series.

Patrick