Re: [PATCH 2/5] http: use `size_t` for byte position in fwrite_sha1_file

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 16, 2026, 14:30 UTC
Message-ID: <xmqq1p9pk99q.fsf@gitster.g>
In-Reply-To: <atHiiye4oi_lF5FP@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 26 quoted lines

> On Wed, Oct 14, 2026 at 06:09:28AM +0000, Johannes Schindelin via GitGitGadget wrote:
>> diff --git a/http.c b/http.c
>> index 4af3c29c76..809471f436 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -2848,7 +2848,7 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
>>  {
>>  	unsigned char expn[4096];
>>  	size_t size = eltsize * nmemb;
>> -	int posn = 0;
>> +	size_t posn = 0;
>>  	struct http_object_request *freq = data;
>>  	struct active_request_slot *slot = freq->slot;
>
> Right. This is how we access the variable:
>
> 	do {
> 		ssize_t retval = xwrite(freq->localfile,
> 					(char *) ptr + posn, size - posn);
> 		if (retval < 0)
> 			return posn / eltsize;
> 		posn += retval;
> 	} while (posn < size);
>
> So it's clear that it doesn't have to be signed either, as we only ever
> add positive values to it.

I agree that changing the type of posn from int to size_t is the right move.

But I wondered what happens when (size - posn) is larger than half the range of the ssize_t type early in the loop. After all, the proposed commit log message worries about the case where HTTP response body exceeds 2GB.

Shouldn't the loop clamp the bytecount passed to xwrite() below whatever would fit in ssize_t when successful? It may end up making two calls of xwrite, writing 1.5GB each, to write out 3GB payload, but that would be better than attempting to write 3GB, succeed, and be fooled by ssize_t that is narrower by size_t by at least one bit that we got a negative return from xwrite() and return an error from here, when in reality we have written them all?

    ... goes and looks ...

Ah, nevermind. The big comment in git-compat-util.h before MAX_IO_SIZE clarifies all of the above. In short, xwrite() takes care of clamping, so its callers including this code do not have to worry about it.