Re: [PATCH 3/6] rust/varint: add safety comments
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 8, 2025, 04:46 UTC
- Message-ID
- <aOXsmIu1BEWzxlVE@pks.im>
- In-Reply-To
- <aOWwcqyithDKQzVs@fruit.crustytoothpaste.net>
On Wed, Oct 08, 2025 at 12:29:38AM +0000, brian m. carlson wrote:
Show 15 quoted lines
> 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.
Show 6 quoted lines
> 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