From: Ben Knoble <ben.knoble@gmail.com>
Date: Tue, 09 Sep 2025 00:49:45 GMT
Subject: Re: [PATCH RFC 2/3] rust: implement a test balloon via the "varint" subsystem
Message-ID: <EF337F13-64D2-4A17-BF7D-FE77E3064E35@gmail.com>
In-Reply-To: <xmqq8qipzhg3.fsf@gitster.g>


> 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.