Re: [PATCH v2 1/1] zlib.c: use size_t for size

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

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