Re: [PATCH v3 11/14] odb: introduce mtime fields for object info requests

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 23, 2026, 17:48 UTC
Message-ID: <aXO0cNaY3DWu6aQ2@nand.local>
In-Reply-To: <aXNCq8h94i2Z6uSa@pks.im>

On Fri, Jan 23, 2026 at 10:43:07AM +0100, Patrick Steinhardt wrote:

Show 20 quoted lines

> > > diff --git a/odb.c b/odb.c
> > > index 65f0447aa5..67decd3908 100644
> > > --- a/odb.c
> > > +++ b/odb.c
> > > @@ -702,6 +702,8 @@ static int do_oid_object_info_extended(struct object_database *odb,
> > >  				oidclr(oi->delta_base_oid, odb->repo->hash_algo);
> > >  			if (oi->contentp)
> > >  				*oi->contentp = xmemdupz(co->buf, co->size);
> > > +			if (oi->mtimep)
> > > +				*oi->mtimep = 0;
> >
> > Assuming that you do not change the object_info request/response
> > semantics, I wonder if it might make sense to zero out the entirety of
> > the response section as a belt-and-suspenders mechanism in case future
> > contributors forget to assign zero to the new fields themselves.
>
> Splitting up the request/response structure as you proposed in a
> previous patch could definitely help with this. I'd prefer to rather do
> such a bigger change as a follow-up though as it would lead to a lot of
> churn.

I'm OK with pushing the larger change down the road, but I am a little uncomfortable with the interim state being introduced here. Perhaps a compromise here would be to have the caller supply a pointer to an object_info struct, whose request fields we honor. The response fields would then be written into a separate object_info struct via an out-parameter.

I don't know. I think that ^ this suggestion is kind of ugly, but I'm trying to come up with something that doesn't introduce the risk I described above in the interim between this patch series and the one you're proposing later on.

Thanks, Taylor