Re: [PATCH RFC 2/3] rust: implement a test balloon via the "varint" subsystem

[Junio C Hamano <gitster@pobox.com>]

Ben Knoble <ben.knoble@gmail.com> writes:

>> Le 8 sept. 2025 à 00:39, Junio C Hamano <gitster@pobox.com> a écrit :
>> 
>> Ben Knoble <ben.knoble@gmail.com> 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.