On 14/10/18 03:52, Jeff King wrote:
> On Sun, Oct 14, 2018 at 03:16:36AM +0100, Ramsay Jones wrote:
>
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index b059b86aee..3b5f2c38b3 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,
>> off_t len)
>> {
>> unsigned char *in;
>> - unsigned long avail;
>> + size_t avail;
>>
>> while (len) {
>> in = use_pack(p, w_curs, offset, &avail);
>> if (avail > len)
>> - avail = (unsigned long)len;
>> + avail = xsize_t(len);
>
> We don't actually care about truncation here. The idea is that we take a
> bite-sized chunk via use_pack, and loop as necessary. So mod-2^32
> truncation via a cast would be bad (we might not make forward progress),
> but truncating to SIZE_MAX would be fine.
>
> That said, we know that would not truncate here, because we must
> strictly be shrinking "avail", which was already a size_t (unless "len"
> is negative, but then we are really screwed ;) ).
>
> So I kind of wonder if a comment would be better than xsize_t here.
> Something like:
>
> if (avail > len) {
> /*
> * This can never truncate because we know that len is smaller
> * than avail, which is already a size_t.
> */
> avail = (size_t)len;
> }
Heh, you are, of course, correct! (that will learn me[1]). :-D
Hmm, let's see if I can muster the enthusiasm to do all that
testing again!
ATB,
Ramsay Jones
[1] Since I started with my patch, when I had finished 'paring
it back', the result didn't have this xsize_t() call. In order
to make the result 'v2 + SZEDER's patch' (which I thought was
quite neat) I added this call right at the end. :-P
