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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 8, 2013 at 1:50 PM, Brandon Casey <drafnel@xxxxxxxxx> wrote:
> On Sun, Jul 7, 2013 at 3:14 AM, Jeff King <peff@xxxxxxxx> wrote:

>> diff --git a/pack-revindex.c b/pack-revindex.c
>> index 77a0465..d2adf36 100644
>> --- a/pack-revindex.c
>> +++ b/pack-revindex.c
>> @@ -59,11 +59,78 @@ static int cmp_offset(const void *a_, const void *b_)
>>         /* revindex elements are lazily initialized */
>>  }
>>
>> -static int cmp_offset(const void *a_, const void *b_)
>> +/*
>> + * This is a least-significant-digit radix sort using a 16-bit "digit".
>> + */
>> +static void sort_revindex(struct revindex_entry *entries, int n, off_t max)
>
> 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.

-Brandon
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]