Re: [PATCH 01/13] refs: introduce wrapper struct for `each_ref_fn`

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 07, 2025 at 01:05:15PM -0500, Justin Tobler wrote:
> On 25/10/07 12:58PM, Patrick Steinhardt wrote:
> > The `each_ref_fn` callback function type is used across our code base
> > for several different functions that iterate through reference. There's
> > a bunch of callbacks implementing this type, which makes any changes to
> > the callback signature extremely noisy. An example of the required churn
> > is e8207717f1 (refs: add referent to each_ref_fn, 2024-08-09): adding a
> > single argument required us to change 48 files.
> > 
> > It was already proposed back then [1] that we might want to introduce a
> > wrapper structure to alleviate the pain going forward. While this of
> > course requires the same kind of global refactoring as just introducing
> > a new parameter, it at least allows us to more change the callback type
> > afterwards by just extending the wrapper structure.
> > 
> > One counterargument to this refactoring is that it makes the structure
> > more opaque. While it is obvious which callsites need to be fixed up
> > when we change the function type, it's not obvious anymore once we use
> > a structure. That being said, we only have a handful of sites that
> > actually need to populate this wrapper structure: our ref backends and
> > "refs/iterator.c".
> 
> It looks like we also populate `stuct reference` in a couple other spots
> where we invoke the callback explicitly.

Fair, let me mention that.

[snip]
> >  /*
> >   * The signature for the callback function for the for_each_*()
> >   * functions below.  The memory pointed to by the refname and oid
> >   * arguments is only guaranteed to be valid for the duration of a
> >   * single callback invocation.
> >   */
> 
> Should we update this comment now that these fields are contained the
> wrapper struct?

Yeah, will do.

Patrick