On Thu, Aug 29, 2024 at 02:58:19PM -0400, Taylor Blau wrote: > On Tue, Aug 27, 2024 at 05:14:14PM -0700, Junio C Hamano wrote: > > > diff --git a/midx.c b/midx.c > > > index ca98bfd7c6..67e0d64004 100644 > > > --- a/midx.c > > > +++ b/midx.c > > > @@ -496,6 +496,7 @@ int nth_bitmapped_pack(struct repository *r, struct multi_pack_index *m, > > > MIDX_CHUNK_BITMAPPED_PACKS_WIDTH * local_pack_int_id + > > > sizeof(uint32_t)); > > > bp->pack_int_id = pack_int_id; > > > + bp->from_midx = m; > > > > Do multi_pack_index objects live as long as bitmapped_pack objects > > that point at them live? If m later goes away without letting the > > bitmapped_pack know about it, the borrowed pointer in from_midx > > would become dangling, which is not what we want to see. > > > > "None of these objects are released or relocated while we are > > running pack-objects, so once the .from_midx member is assigned > > here, it will always be pointing at a valid multi_pack_index object" > > is a satisfactory answer, I guess. > > Good question, and good answer ;-). > > This is only relevant in a read-only path where we're generating a new > pack from existing packs and not altering those pack or rewriting / > deleting the MIDX attached to them. So I think we're OK here and don't > have any lifetime/scope issues. Do we ever close/reopen the midx? For example, if a simultaneous process wrote a new one and we triggered reprepare_packed_git()? I think the answer is "no"; we might read a new midx, but we never ditch the old one (just like we do for packed_git structs). Which I suspect is needed even before this patch, since various other parts of the bitmap code (and probably others) rely on the struct staying in place across as we read many objects. -Peff
