Re: [PATCH v3 08/14] builtin/fsck: refactor to use `odb_for_each_object()`
On Thu, Jan 22, 2026 at 07:32:32PM -0500, Taylor Blau wrote:
Show 14 quoted lines
> On Wed, Jan 21, 2026 at 01:50:24PM +0100, Patrick Steinhardt wrote:
> > diff --git a/builtin/fsck.c b/builtin/fsck.c
> > index 4979bc795e..96107695ae 100644
> > --- a/builtin/fsck.c
> > +++ b/builtin/fsck.c
> > @@ -218,15 +218,17 @@ static int mark_used(struct object *obj, enum object_type type UNUSED,
> > return 0;
> > }
> >
> > -static void mark_unreachable_referents(const struct object_id *oid)
> > +static int mark_unreachable_referents(const struct object_id *oid,
> > + struct object_info *io UNUSED,
>
> s/io/oi/ ?
Show 17 quoted lines
> > + void *data UNUSED)
> > {
>
> The transformation here makes sense (and I think aloud through the
> similar mark_object_for_connectivity() transformation below). One
> thought that I had while reading, though, was how this function behaves
> when it is passed the same object more than once, since you mention that
> as a possibility in the commit which introduces odb_for_each_object().
>
> I think that this is OK, since we already likely send the same object to
> this function multiple times if, e.g., we freshened an object from the
> cruft pack, in which case we'd see it both when iterating packed
> objects as well as when iterating loose ones.
>
> As far as I can tell, that's OK, but it might be nice to provide a brief
> analysis of that in the commit message, just to be sure and to help
> future readers.Fair point, will mention.
Patrick