On Tue, Oct 10, 2017 at 09:56:38PM +0900, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > OK, I think that makes more sense. But note the p->num_objects thing I > > mentioned. If I do: > > > > git pack-objects .git/objects/pack/pack </dev/null > > > > then I have a pack with zero objects, which I think we'd similarly want > > to return early from. I.e., I think we need: > > > > if (p->num_objects) > > return; > > > > Technically that also covers open_pack_index() failure, too, but that's > > a subtlety I don't think we should rely on. > > True. I notice that the early part of the two functions look almost > identical. Do we need error condition handling for the other one, > too? I'm not sure which two you mean. Do you mean find_pack_entry_one() in packfile.c as the other one? If so, I think it is fine in the zero-object case, because it does not do the "this is the sha1 at the position where it _would_ be found" trick, which is what causes us to potentially dereference nonsense. -Peff