From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Wed, 01 Oct 2025 17:21:25 GMT
Subject: Re: [PATCH v7 6/9] varint: reimplement as test balloon for Rust
Message-ID: <CAH=ZcbC6jMUBMLrmwzksAzLM3t=XH+hBnf+=wLdjAcAiWTx7vw@mail.gmail.com>
In-Reply-To: <20250925-b4-pks-rust-breaking-change-v7-6-4e49dcb904d5@pks.im>

On Thu, Sep 25, 2025 at 12:30 AM Patrick Steinhardt <ps@pks.im> wrote:
>  Makefile        |  3 ++
>  meson.build     |  5 +++-
>  src/lib.rs      |  1 +
>  src/meson.build |  1 +
>  src/varint.rs   | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 31e79342e1d..2a7fc5cb1f3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1307,7 +1307,9 @@ LIB_OBJS += urlmatch.o
>  LIB_OBJS += usage.o
>  LIB_OBJS += userdiff.o
>  LIB_OBJS += utf8.o
> +ifndef WITH_RUST
>  LIB_OBJS += varint.o
> +endif
>  LIB_OBJS += version.o
>  LIB_OBJS += versioncmp.o
>  LIB_OBJS += walker.o
> @@ -1499,6 +1501,7 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
>  UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
>
>  RUST_SOURCES += src/lib.rs
> +RUST_SOURCES += src/varint.rs
>
>  GIT-VERSION-FILE: FORCE
>         @OLD=$$(cat $@ 2>/dev/null || :) && \
> diff --git a/meson.build b/meson.build
> index 234a9e9d6fd..37dfa286017 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -522,7 +522,6 @@ libgit_sources = [
>    'usage.c',
>    'userdiff.c',
>    'utf8.c',
> -  'varint.c',
>    'version.c',
>    'versioncmp.c',
>    'walker.c',
> @@ -1707,6 +1706,10 @@ rust_option = get_option('rust').disable_auto_if(not cargo.found())
>  if rust_option.allowed()
>    subdir('src')
>    libgit_c_args += '-DWITH_RUST'
> +else
> +  libgit_sources += [
> +    'varint.c',
> +  ]
>  endif
>
>  libgit = declare_dependency(
> diff --git a/src/lib.rs b/src/lib.rs
> index e69de29bb2d..9da70d8b57d 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -0,0 +1 @@
> +pub mod varint;
> diff --git a/src/meson.build b/src/meson.build
> index c8d874b2106..25b9ad5a147 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -1,5 +1,6 @@
>  libgit_rs_sources = [
>    'lib.rs',
> +  'varint.rs',
>  ]
>
>  # Unfortunately we must use a wrapper command to move the output file into the
> diff --git a/src/varint.rs b/src/varint.rs
> new file mode 100644
> index 00000000000..6e610bdd8e0
> --- /dev/null
> +++ b/src/varint.rs
> @@ -0,0 +1,92 @@
> +#[no_mangle]
> +pub unsafe extern "C" fn decode_varint(bufp: *mut *const u8) -> u64 {
> +    let mut buf = *bufp;
> +    let mut c = *buf;
> +    let mut val = u64::from(c & 127);
> +
> +    buf = buf.add(1);
> +
> +    while (c & 128) != 0 {
> +        val = val.wrapping_add(1);
> +        if val == 0 || val.leading_zeros() < 7 {
> +            return 0; // overflow
> +        }
> +
> +        c = *buf;
> +        buf = buf.add(1);
> +
> +        val = (val << 7) + u64::from(c & 127);
> +    }
> +
> +    *bufp = buf;
> +    val
> +}
> +
> +#[no_mangle]
> +pub unsafe extern "C" fn encode_varint(value: u64, buf: *mut u8) -> u8 {
> +    let mut varint: [u8; 16] = [0; 16];
> +    let mut pos = varint.len() - 1;
> +
> +    varint[pos] = (value & 127) as u8;
> +
> +    let mut value = value >> 7;
> +    while value != 0 {
> +        pos -= 1;
> +        value -= 1;
> +        varint[pos] = 128 | (value & 127) as u8;
> +        value >>= 7;
> +    }
> +
> +    if !buf.is_null() {
> +        std::ptr::copy_nonoverlapping(varint.as_ptr().add(pos), buf, varint.len() - pos);
> +    }
> +
> +    (varint.len() - pos) as u8
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use super::*;
> +
> +    #[test]
> +    fn test_decode_varint() {
> +        unsafe {
> +            assert_eq!(decode_varint(&mut [0x00].as_slice().as_ptr()), 0);
> +            assert_eq!(decode_varint(&mut [0x01].as_slice().as_ptr()), 1);
> +            assert_eq!(decode_varint(&mut [0x7f].as_slice().as_ptr()), 127);
> +            assert_eq!(decode_varint(&mut [0x80, 0x00].as_slice().as_ptr()), 128);
> +            assert_eq!(decode_varint(&mut [0x80, 0x01].as_slice().as_ptr()), 129);
> +            assert_eq!(decode_varint(&mut [0x80, 0x7f].as_slice().as_ptr()), 255);
> +
> +            // Overflows are expected to return 0.
> +            assert_eq!(decode_varint(&mut [0x88; 16].as_slice().as_ptr()), 0);
> +        }
> +    }
> +
> +    #[test]
> +    fn test_encode_varint() {
> +        unsafe {
> +            let mut varint: [u8; 16] = [0; 16];
> +
> +            assert_eq!(encode_varint(0, std::ptr::null_mut()), 1);
> +
> +            assert_eq!(encode_varint(0, varint.as_mut_slice().as_mut_ptr()), 1);
> +            assert_eq!(varint, [0; 16]);
> +
> +            assert_eq!(encode_varint(10, varint.as_mut_slice().as_mut_ptr()), 1);
> +            assert_eq!(varint, [10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
> +
> +            assert_eq!(encode_varint(127, varint.as_mut_slice().as_mut_ptr()), 1);
> +            assert_eq!(varint, [127, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
> +
> +            assert_eq!(encode_varint(128, varint.as_mut_slice().as_mut_ptr()), 2);
> +            assert_eq!(varint, [128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
> +
> +            assert_eq!(encode_varint(129, varint.as_mut_slice().as_mut_ptr()), 2);
> +            assert_eq!(varint, [128, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
> +
> +            assert_eq!(encode_varint(255, varint.as_mut_slice().as_mut_ptr()), 2);
> +            assert_eq!(varint, [128, 127, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
> +        }
> +    }
> +}

Looks good.