Subject: Re: [PATCH 01/16] packfile: introduce a new `struct packfile_store`

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 20 Aug 2025 06:58:24 +0200

On Tue, Aug 19, 2025 at 10:32:20AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> >  	memset(o, 0, sizeof(*o));
> >  	o->repo = repo;
> > +	o->packfiles = packfile_store_new(o);
> 
> Shouldn't this be called o->packfile_store?  It is not like a
> packfile_store is merely an array of packfile struct, is it?

I was mostly aiming for brevity as there's going to be a bunch of sites
that access the variable. But if you feel strongly about it I can adapt.

> > @@ -128,6 +129,7 @@ struct object_database {
> >  	 *
> >  	 * should only be accessed directly by packfile.c
> >  	 */
> > +	struct packfile_store *packfiles;
> 
> So odb has a pointer to packfile_store, which in turn has a pointer
> to a(nother) odb?

The ODB has a pointer to a packfile store, and that store has a pointer
to its owning ODB.

> Hmph.  It is unclear what this step has achieved (in other words,
> there is no obvious thing that the information stored in the new
> structure is used to achieve at this step).  Let me read on.

Well, this series really only cares about encapsulating access to
packfiles so that it becomes easier in subsequent patch series to move
the data structures around. So next to carving out the subsystem we
don't achieve much, but it's a necessary prerequisite.

Patrick