Re: [PATCH] git-compat-util: introduce `count_t` typedef
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 7, 2025, 16:43 UTC
- Message-ID
- <xmqq4iujqdzm.fsf@gitster.g>
- In-Reply-To
- <582e8e75-c6eb-4845-8f3b-62f234f0964f@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 14 quoted lines
> Hi Patrick > > On 07/08/2025 10:22, Patrick Steinhardt wrote: >> Historically, Git has been very lenient with its use of integer types >> and didn't really give much thought into which type to use in what >> situation. We interchangeably mix and match signed and unsigned types >> and often times blindly convert them. This use has led to several >> out-of-bounds reads and writes in the past, some of which could be >> turned into arbitrary code execution. > > My feeling is that one of the main problems has been using different > types for loop indexes and loop limits. If we mandated that the loop > index had to be the same type as the limit that would improve things > considerably and without mandating a particular type.
Yup. And the limit being unsigned would force the counter to be also unsigned, which can introduce buggy constructs (like counting down).
Show 13 quoted lines
>> A discussion that regularly comes up in this context though is what >> types to use for counting entities: >> - One question is whether the type should be signed or unsigned. >> Arguably, the answer should be to use unsigned types as long as we >> know that we never need a negative value, e.g. as a sentinel. This >> helps guide the reader and explicitly conveys the sense that such a >> counter is only ever going to be a non-negative number. Otherwise, >> code would need to be more careful as it may hold negative values. > > The counter argument to this is that it is easy to write incorrect > loops when counting down if the loop variable is unsigned. Using a > typedef that hides the actual type makes that harder to spot as it is > not immediately obvious whether the loop index is signed or not.
This is very valid argument against typedef for something trivial like an integer. Use of proposed count_t loses both size and signedness information.
Thanks.