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:
> > 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.
> > + }
> > + 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
