From: Taylor Blau <me@ttaylorr.com>
Date: Fri, 23 Jan 2026 17:48:32 GMT
Subject: Re: [PATCH v3 11/14] odb: introduce mtime fields for object info requests
Message-ID: <aXO0cNaY3DWu6aQ2@nand.local>
In-Reply-To: <aXNCq8h94i2Z6uSa@pks.im>

On Fri, Jan 23, 2026 at 10:43:07AM +0100, Patrick Steinhardt wrote:
> > > 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