Re: [PATCH v3 1/7] object-file: always set OI_LOOSE when reading object info

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 7, 2026, 11:27 UTC
Message-ID: <aV5DHI04KBs4GJn-@pks.im>
In-Reply-To: <CAOLa=ZSNmi_Lzb=3EdWks=mMOPvfijT2659y4YtxWnUKVUOXaA@mail.gmail.com>

On Wed, Jan 07, 2026 at 12:50:45AM -0800, Karthik Nayak wrote:

Show 37 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/object-file.c b/object-file.c
> > index 6280e42f34..d566df427a 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,
> >  	 */
> >  	if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
> >  		struct stat st;
> > -		if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
> > -			return quick_has_loose(source->loose, oid) ? 0 : -1;
> > +
> > +		if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {
> > +			status = quick_has_loose(source->loose, oid) ? 0 : -1;
> > +			if (!status && oi)
> > +				oi->whence = OI_LOOSE;
> > +			return status;
> > +		}
> > +
> >  		if (stat_loose_object(source->loose, oid, &st, &path) < 0)
> >  			return -1;
> > -		if (oi && oi->disk_sizep)
> > -			*oi->disk_sizep = st.st_size;
> > +
> > +		if (oi) {
> > +			if (oi->disk_sizep)
> > +				*oi->disk_sizep = st.st_size;
> > +			oi->whence = OI_LOOSE;
> > +		}
> > +
> >  		return 0;
> >  	}
> >
> 
> The change looks good. I'm wary of early returns independently doing the
> cleanup, wonder if it'd be better to do `status = ...; goto cleanup`
> instead.

I share that sentiment, and I was in fact having a look at what it would take to have a single exit path in this function. I eventually discarded the work though because it required a bunch of changes to really make this whole function more readable than it currently is.

But now that you're the second one thinking this I'll probably bite the bullet and just do it.

Thanks!

Patrick