Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Oct 4, 2019, 17:09 UTC
- Message-ID
- <ae6a64f4-8f46-cbaf-5004-defc316c5157@kdbg.org>
- In-Reply-To
- <nycvar.QRO.7.76.6.1910041154120.46@tvgsbejvaqbjf.bet>
Am 04.10.19 um 11:55 schrieb Johannes Schindelin:
Show 23 quoted lines
> On Fri, 4 Oct 2019, Junio C Hamano wrote:
>> These three look good and too similar to each other, which makes me
>> wonder if we want to allow them simply write
>>
>> return insert_pos_as_negative_offset(nr);
>>
>> with something like
>>
>> static int insert_pos_as_negative_offset(uintmax_t nr)
>> {
>> if (INT_MAX < nr)
>> die("overflow: -1 - %"PRIuMAX, nr);
>> return -1 - (int)nr;
>> }
>>
>> to avoid repetition.
>
> I tried not to do that because there are two different data types in
> play: `unsigned int` and `size_t`. But I guess by making this an
> `inline` function, compilers can optimize for the common case and avoid
> casting _twice_.
>
> Will be fixed in v2,IMHO, if you don't accompany insert_pos_as_negative_offset() with a corresponding extract_pos_and_found_condition() and use it everywhere, it is more obfuscating than necessary.
The *real* problem to solve is to ensure that the index/cache does not grow so large that 32-bit indexes would be needed. Then the calculation that you want to hide here cannot overflow.
-- Hannes