Message-ID: <CAGSQo01LXCY2zf6vP5r1yP+FWT9ZKbOARJC=HpHCq=5HY3jajA@mail.gmail.com>
Date: Fri, 26 Dec 2025 12:27:29 -0800
From: Matthew Maurer <mmaurer@...gle.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] rust: transmute: Support transmuting slices of
 AsBytes/FromBytes types

On Wed, Dec 17, 2025 at 8:52 AM Daniel Almeida
<daniel.almeida@...labora.com> wrote:
>
> Hi Matthew,
>
> > On 15 Dec 2025, at 21:44, Matthew Maurer <mmaurer@...gle.com> wrote:
> >
> > Currently, we support either transmuting a byte slice of the exact size
> > of the target type or a single element from a prefix of a byte slice.
> >
> > This adds support for transmuting from a byte slice to a slice of
> > elements, assuming appropriate traits are implemented.
> >
> > Signed-off-by: Matthew Maurer <mmaurer@...gle.com>
> > ---
> > rust/kernel/transmute.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 72 insertions(+)
> >
> > diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> > index be5dbf3829e25c92f85083cc0f4940f35bb7baec..5cc5f4fde4c31cea3c51ab078b4d68f3a0a2caa1 100644
> > --- a/rust/kernel/transmute.rs
> > +++ b/rust/kernel/transmute.rs
> > @@ -122,6 +122,78 @@ fn from_bytes_mut_prefix(bytes: &mut [u8]) -> Option<(&mut Self, &mut [u8])>
> >         }
> >     }
> >
> > +    /// Converts a slice of bytes to a slice of `Self`.
> > +    ///
> > +    /// Succeeds if the reference is properly aligned, and the size of `bytes` is a multiple of that
> > +    /// of `Self`.
> > +    ///
> > +    /// Otherwise, returns [`None`].
> > +    fn from_bytes_to_slice(bytes: &[u8]) -> Option<&[Self]>
> > +    where
> > +        Self: Sized,
> > +    {
> > +        let size = size_of::<Self>();
> > +        if size == 0 {
> > +            return None;
> > +        }
> > +        if bytes.len() % size != 0 {
> > +            return None;
> > +        }
> > +        let len = bytes.len() / size;
> > +        let ptr = bytes.as_ptr().cast::<Self>();
> > +
> > +        #[allow(clippy::incompatible_msrv)]
> > +        if ptr.is_aligned() {
> > +            // SAFETY:
> > +            // - `ptr` is valid for reads of `bytes.len()` bytes, which is
> > +            //   `len * size_of::<Self>()`.
> > +            // - `ptr` is aligned for `Self`.
> > +            // - `ptr` points to `len` consecutive properly initialized values of type `Self`
> > +            //   because `Self` implements `FromBytes` (any bit pattern is valid).
> > +            // - The lifetime of the returned slice is bound to the input `bytes`.
> > +            unsafe { Some(core::slice::from_raw_parts(ptr, len)) }
> > +        } else {
> > +            None
> > +        }
>
> Nit: I’d rewrite this as
>
> if !ptr.is_aligned() {
>  return None;
> }
>
> unsafe { Some(core::slice::from_raw_parts(ptr, len)) }
>
> This saves one indentation level.
>

If this code were sizable, or if there were no unsafe, I'd agree with
you. However:
1. It makes the path condition for the `unsafe` block clearer - the if
condition directly guards the unsafe block, which makes it locally
obvious why it can assume "`ptr` is aligned for `Self`." With the
"check error and early return" strategy, you need to reason less
locally about the reachability of the statement.
2. The guarded code is one line long. If it were significant, then I
can see the point of using an early-return style in order to avoid
indentation, especially if the block *itself* had indentation. This is
one line.
3. This is coherent with `from_bytes_mut` and `from_bytes` in this
file, which also use this style.

> > +    }
> > +
> > +    /// Converts a mutable slice of bytes to a mutable slice of `Self`.
> > +    ///
> > +    /// Succeeds if the reference is properly aligned, and the size of `bytes` is a multiple of that
> > +    /// of `Self`.
> > +    ///
> > +    /// Otherwise, returns [`None`].
> > +    fn from_bytes_to_mut_slice(bytes: &mut [u8]) -> Option<&mut [Self]>
> > +    where
> > +        Self: AsBytes + Sized,
> > +    {
> > +        let size = size_of::<Self>();
> > +        if size == 0 {
> > +            return None;
> > +        }
> > +        if bytes.len() % size != 0 {
> > +            return None;
> > +        }
> > +        let len = bytes.len() / size;
> > +        let ptr = bytes.as_mut_ptr().cast::<Self>();
> > +
> > +        #[allow(clippy::incompatible_msrv)]
> > +        if ptr.is_aligned() {
> > +            // SAFETY:
> > +            // - `ptr` is valid for reads and writes of `bytes.len()` bytes, which is
> > +            //   `len * size_of::<Self>()`.
> > +            // - `ptr` is aligned for `Self`.
> > +            // - `ptr` points to `len` consecutive properly initialized values of type `Self`
> > +            //   because `Self` implements `FromBytes`.
> > +            // - `AsBytes` guarantees that writing a new `Self` to the resulting slice can safely
> > +            //   be reflected as bytes in the original slice.
> > +            // - The lifetime of the returned slice is bound to the input `bytes`.
> > +            unsafe { Some(core::slice::from_raw_parts_mut(ptr, len)) }
> > +        } else {
> > +            None
> > +        }
>
> Same here.

Same here :)

>
> > +    }
> > +
> >     /// Creates an owned instance of `Self` by copying `bytes`.
> >     ///
> >     /// Unlike [`FromBytes::from_bytes`], which requires aligned input, this method can be used on
> >
> > --
> > 2.52.0.305.g3fc767764a-goog
> >
> >
>
> Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
>

Powered by blists - more mailing lists