On Tue, Oct 10, 2017 at 09:11:15AM -0400, Derrick Stolee wrote: > On 10/10/2017 8:56 AM, 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 prefer to fix the problem in all code clones when they cause review > friction, so I'll send a fifth commit showing just the diff for these > packfile issues in sha1_name.c. See patch below. Ah, that answers my earlier question. Junio mean unique_in_pack(). And yeah, I think it suffers from the same problem. > Should open_pack_index() return a non-zero status if the packfile is empty? > Or, is there a meaningful reason to have empty packfiles? I can't think of a compelling reason to have an empty packfile. But nor do I think we should consider them an error when we can handle them quietly (and returning non-zero status would cause Git to complain on many operations in a repository that has such a file). -Peff
