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

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Fri, 12 Oct 2018, Junio C Hamano wrote:
>
>> From: Martin Koegler <martin.koegler@chello.at>
>> Date: Thu, 10 Aug 2017 20:13:08 +0200
>> 
>> Signed-off-by: Martin Koegler <martin.koegler@chello.at>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> 
>>  * I made minimal adjustments to make the change apply to today's
>>    codebase.  I still find some choices and mixing of off_t and
>>    size_t done by the patch a bit iffy, and some arith may need to
>>    become st_addX().  Extra set of eyes are very much appreciated.
>> 
>>  builtin/pack-objects.c | 10 +++++-----
>>  cache.h                | 10 +++++-----
>>  pack-check.c           |  6 +++---
>>  pack.h                 |  2 +-
>>  packfile.h             |  2 +-
>>  wrapper.c              |  8 ++++----
>>  zlib.c                 |  8 ++++----
>>  7 files changed, 23 insertions(+), 23 deletions(-)
>> 
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index e6316d294d..b9ca04eb8a 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,
>>  		struct packed_git *p,
>>  		struct pack_window **w_curs,
>>  		off_t offset,
>> -		off_t len)
>> +		size_t len)
>>  {
>>  	unsigned char *in;
>> -	unsigned long avail;
>> +	size_t avail;
>>  
>>  	while (len) {
>>  		in = use_pack(p, w_curs, offset, &avail);
>>  		if (avail > len)
>
> Do we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I
> guess we would receive a compiler warning about different sizes in that
> case, but it makes me worry.

Yup, you just said exactly the same thing as I already said in the
part you quoted.  I still find the mixed use of off_t and size_t in
this patch iffy, and that was the secondary reason why the patch was
kept in the stalled state for so long.  The primary reason was that
nobody tried to dust it off and reignite the topic so far---which I
am trying to correct, but as I said, this is just minimally adjusted
to today's codebase, without any attempt to improve relative to the
original patch.

I think we are in agreement in that this is not making things worse,
as we are already in the mixed arith territory before this patch.