Re: [PATCH 2/4] object-file: adapt `stream_object_signature()` to take a stream

> > That matches the existing code (since it all happened in a
> > single function), but should we take this opportunity to give more
> > accurate error messages? I.e., to do:
> > 
> >   if (!stream) {
> > 	error(_("unable to open object stream for %s"), oid_to_hex(oid));
> > 	return NULL;
> >   }
> >   if (stream_object_signature(r, stream, repl) < 0) {
> > 	error(_("hash mismatch %s"), oid_to_hex(oid));
> > 	odb_read_stream_close(stream);
> > 	return NULL;
> >   }
> >   odb_read_stream_close(stream);
> > 
> > I dunno. It should be quite uncommon to see either of these messages,
> > but that is sometimes the moment when details are most important.
> 
> Agreed, that feels like a sensible change indeed. Also makes the code
> flow easier to follow in my opinion.

> > Also, as an aside, I found it curious that we still need to pass the
> > repository struct to stream_object_signature(). That's because it needs
> > to know the correct hash_algo. I wondered if the stream struct itself
> > might know about that, but it doesn't seem to (it doesn't know anything
> > about where it came from). So it's unavoidable that we'd need to retain
> > it.
> 
> Yeah, agreed. I wondered whether we should eventually extend `struct
> odb_read_stream` to have a pointer to the owning object source, and in
> that case we could've avoided the extra repository parameter. But I
> decided it was out of scope for this patch series, also because I don't
> want to cause conflicts with other stuff I'm working on in this vicinity
> :)