Re: [PATCH 3/6] rust/varint: add safety comments

[Patrick Steinhardt <ps@pks.im>]

On Wed, Oct 08, 2025 at 12:29:38AM +0000, brian m. carlson wrote:
> On 2025-10-07 at 12:36:31, Patrick Steinhardt wrote:
> > +/// # Safety
> > +///
> > +/// The provided buffer must be large enough to store the encoded varint. Callers may either provide
> > +/// a `[u8; 16]` here, which is guaranteed to satisfy all encodable numbers. Or they can call this
> > +/// function with a `NULL` pointer first to figure out array size.
> >  #[no_mangle]
> >  pub unsafe extern "C" fn encode_varint(value: u64, buf: *mut u8) -> u8 {
> >      let mut varint: [u8; 16] = [0; 16];
> 
> I'm planning to do something a little different with this code by
> refactoring it out into a Rust function, so at that point it will no
> longer be possible to provide a buffer smaller than 16 bytes.  Note that
> all callers of this function pass a 16-byte buffer, so that should be
> safe.

Ah, true. I just double-checked, and all callers pass in a 16 byte
buffer indeed. Also means that the NULL-pointer handling can go away in
theory, as we don't use it.

In any case, your direction makes sense once we have Rust-internal
callers of this functionality. We definitely don't want to propagate the
unsafety to callers and should make sure that it is contained to the C
API.

> That doesn't mean that you can't send this patch (and I think your patch
> is good), just that we shouldn't tell people we can use a buffer smaller
> than 16 bytes, since that will at some point no longer be true.
> 
> Here's the current version of the patch I'm planning on sending for
> reference.  I can rebase onto your series once Junio picks it up.

The patch makes sense to me, thanks. I'll not pick it up yet though as
there is no justifiable need as part of my series, but I'm happy to
adjust the comment.

Thanks!

Patrick