Re: [PATCH 10/17] odb/source: make `read_object_stream()` function pluggable

[Patrick Steinhardt <ps@pks.im>]

On Thu, Mar 05, 2026 at 11:13:24AM +0000, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/odb/source-files.c b/odb/source-files.c
> > index f2969a1214..b50a1f5492 100644
> > --- a/odb/source-files.c
> > +++ b/odb/source-files.c
> > @@ -55,6 +55,17 @@ static int odb_source_files_read_object_info(struct odb_source *source,
> >  	return -1;
> >  }
> >
> > +static int odb_source_files_read_object_stream(struct odb_read_stream **out,
> > +					       struct odb_source *source,
> > +					       const struct object_id *oid)
> > +{
> > +	struct odb_source_files *files = odb_source_files_downcast(source);
> > +	if (!packfile_store_read_object_stream(out, files->packed, oid) ||
> > +	    !odb_source_loose_read_object_stream(out, source, oid))
> > +		return 0;
> > +	return -1;
> 
> Same issue here regarding loss of error code propagation.

That's fair, but we didn't propagate the exact error code beforehand,
either. Furthermore, it's not even specified what different error codes
would mean, so returning `-1` seems good enough to me. It would also
mean that we only ever propagate error codes from reading loose objects,
not from the packfiles, which would be leaking an implementation detail.

So overall I think this is okay as-is, but let me know in case you
disagree.

Patrick