Ben Knoble <ben.knoble@xxxxxxxxx> writes:
>> Le 8 sept. 2025 à 00:39, Junio C Hamano <gitster@xxxxxxxxx> a écrit :
>>
>> Ben Knoble <ben.knoble@xxxxxxxxx> writes:
>>
>>>> +#[no_mangle]
>>>> +pub unsafe extern "C" fn decode_varint(bufp: *mut *const c_uchar) -> usize {
>>>> + let mut buf = *bufp;
>>>> + let mut c = *buf;
>>>> + let mut val = usize::from(c & 127);
>>>> +
>>>> + buf = buf.add(1);
>>>> +
>>>> + while (c & 128) != 0 {
>>>> + val += 1;
>>>> + if val == 0 || val.leading_zeros() < 7 {
>>>> + return 0; // overflow
>>>
>>> Hm. I thought overflows panic in debug builds, in which case
>>> checking afterwards is too late? Does unsafe change that?
>>
>> This code is a very faithful conversion from C so if somebody does
>> not read Rust well, they can safely refer to the original in C.
>>
>> In either variant, the leading zero's check asks "can we shift val
>> by 7 bits to the left?" _before_ it actually shifts val (and or'es
>> in the lower bits of c), so the "overflow" check is "if we processed
>> any more data we _would_ overflow, so we stop before overflowing".
>>
>> IOW, the code _is_ avoiding the "too late" condition.
>
> Maybe I wasn’t clear, sorry: don’t we already have overflow if
> after val+=1 we also have val==0? In C with unsigned types AFAIK
> that’s the normal modular arithmetic, but I thought I recalled
> that such (unsigned) overflow panics in Rust in debug builds (not
> in release).
>
> So that’s my « checking afterwards » above.
>
> I’ll see if I can double-check my memory though.
Ahh, you meant "can we safely add 1 to val here without
overflowing?"
You probably are correct. If val in the last round had top 7 bits
all 0 and we shifted the lower 7 bits of 'c' in, and if that made
val all 1 bit, then in the modular arithmetic, we may get val==0
after adding 1 to it, but that may indeed be "overflow"ing.
Thanks.
