Re: [PATCH 3/7] builtin/bugreport.c: avoid size_t overflow

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 2, 2022, 20:51 UTC
Message-ID: <xmqqwnbqfjx4.fsf@gitster.g>
In-Reply-To: <c075e483-6e00-5de4-b923-af8983ba384e@github.com>

Victoria Dye <vdye@github.com> writes:

Show 8 quoted lines

> I chose 'uint64_t' to mimic 'throughput_string()' [1], but the signed
> 'off_t' is a better choice given its use in 'strbuf_humanise_bytes()'.
>
> On a related note, while writing this I made the (unsubstantiated)
> assumption that 'off_t' would be a 64-bit int, even on 32-bit systems. Your
> comment prompted me to confirm that assumption; while 'off_t' is not always
> guaranteed to be an int64 by default [2], Git is compiled with '#define
> _FILE_OFFSET_BITS 64' [3] so 'off_t' is equivalent to 'off64_t'.

Offset into a single file can be smaller than the size of the whole disk, after all, so from that point of view, use of off_t in "humanise_bytes" API may be something we would want to fix eventually to reduce confusion.

I do not particularly mind casting up to uint64, especially if that matches the code lifted from scalar. As long as our longer term plan is to update strbuf_humanise_bytes() to take the widest possible unsigned integer, what we do right now would not make that much of a difference.

Thanks.