Re: [PATCH 4/4] pack-revindex: radix-sort the revindex

[Jeff King <peff@peff.net>]

On Mon, Jul 08, 2013 at 02:35:10PM -0700, Brandon Casey wrote:

> > If 'n' is the number of objects in the pack, shouldn't it be unsigned?
> >
> > The data type for struct packed_git.num_objects is uint32_t.  Looks
> > like create_pack_revindex uses the wrong datatype when it captures
> > num_objects in the int num_ent and passes it to sort_revindex.  So, it
> > looks like that function needs to be updated too.
> 
> Hmm.  It seems more than just create_pack_revindex holds num_objects
> in a signed int.  Don't we support 4G objects in packs?
> 
> find_pack_revindex uses a signed int for the index variables in its
> binary search which means it will fail when num_objects >= 2^31.

Yes, I didn't make a test case, but I suspect it is just completely
broken at that scale. But nobody hits it because having 2^31 objects is
somewhat insane (we are still only in the 2^22 range for the kernel, and
most large repos I've seen have large objects, but not necessarily more
of them). So if development keeps up, the kernel should hit the bug in
another few thousand years. :)

-Peff