Jeff King <p...@peff.net> writes: > On Fri, Mar 31, 2017 at 01:45:15PM -0400, Jeff King wrote: > >> I suspect this may improve things, but I haven't dug deeper to see if >> there are unwanted side effects, or if there are other spots that need >> similar treatment. >> >> diff --git a/sha1_file.c b/sha1_file.c >> index 43990dec7..38411f90b 100644 >> --- a/sha1_file.c >> +++ b/sha1_file.c >> @@ -2952,7 +2952,7 @@ static int sha1_loose_object_info(const unsigned char >> *sha1, >> if (status && oi->typep) >> *oi->typep = status; >> strbuf_release(&hdrbuf); >> - return 0; >> + return status; >> } >> >> int sha1_object_info_extended(const unsigned char *sha1, struct object_info >> *oi, unsigned flags) > > Er, no, that's totally wrong. "status' may be holding the type. It > should really be: > > return status < 0 ? status : 0;
Sounds more like it. The only caller will say "ah, that object is not available to us---let's try packs again", which is exactly what we want to happen. There is another bug in the codepath: the assignment to *oi->typep in the pre-context must guard against negative status value. By returning an error correctly like you do above, that bug becomes more or less irrelevant, though.
