[PATCH v10] rust: transmute: Add methods for FromBytes trait

[PATCH v10] rust: transmute: Add methods for FromBytes trait

[Christian S. Lima]

The two methods added take a slice of bytes and return those bytes in
a specific type. These methods are useful when we need to transform
the stream of bytes into specific type.

Since the `is_aligned` method for pointer types has been stabilized in
`1.79` version and is being used in this patch. I'm enabling the
feature. In this case, using this method is useful to check the
alignment and avoid a giant boilerplate, such as `(foo.as_ptr() as
usize) % core::mem::align_of::<T>() == 0`.

Even enabling in `rust/kernel/lib.rs` when compiling with `make LLVM=1
CLIPPY=1` a warning is issued, so in order to compile, it was used
locally the `#[allow(clippy::incompatible_msrv)]`.

Link: https://github.com/Rust-for-Linux/linux/issues/1119
Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Christian S. Lima <christiansantoslima21@gmail.com>
---
Changes in v2:
- Rollback the implementation for the macro in the repository and implement
  methods in trait
- Link to v2: https://lore.kernel.org/rust-for-linux/20241012070121.110481-1-christiansantoslima21@gmail.com/

Changes in v3:
- Fix grammar errors
- Remove repeated tests
- Fix alignment errors
- Fix tests not building
- Link to v3: https://lore.kernel.org/rust-for-linux/20241109055442.85190-1-christiansantoslima21@gmail.com/

Changes in v4:
- Removed core::simd::ToBytes
- Changed trait and methods to safe Add
- Result<&Self, Error> in order to make safe methods
- Link to v4: https://lore.kernel.org/rust-for-linux/20250314034910.134463-1-christiansantoslima21@gmail.com/

Changes in v5:
- Changed from Result to Option
- Removed commentaries
- Returned trait impl to unsafe
- Link to v5: https://lore.kernel.org/rust-for-linux/20250320014041.101470-1-christiansantoslima21@gmail.com/

Changes in v6:
- Add endianess check to doc test and use match to check
success case
- Reformulated safety comments
- Link to v6: https://lore.kernel.org/rust-for-linux/20250330234039.29814-1-christiansantoslima21@gmail.com/

Changes in v7:
- Add alignment check
- Link to v7: https://lore.kernel.org/rust-for-linux/20250615072042.133290-1-christiansantoslima21@gmail.com/

Changes in v8:
- Add the new FromBytesSized trait
- Change the implementation of FromBytes trait methods
- Move the cast to pointer earlier and use `is_aligned()` instead manual
alignment check
- Link to v8: https://lore.kernel.org/rust-for-linux/20250624042802.105623-1-christiansantoslima21@gmail.com/

Changes in v9:
- Improve code comments and remove confusing parts.
- Add a build_assert in the conversion of type `[T]` to check for elements
inside the slice.
- Count the elements in the `[T]` conversion instead of using byte
count.
- Link to v9: https://lore.kernel.org/rust-for-linux/20250811213851.65644-1-christiansantoslima21@gmail.com/#t

Changes in v10:
- Remove `FromBytesSized` trait
- Remove implementation for slice types
- Fix doctest not compiling because `?` operator outside a function
that return `Option<()>`
- Make `FromBytes` trait depend on `Sized`
- Add `is_aligned` as feature
---
 rust/kernel/lib.rs       |  1 +
 rust/kernel/transmute.rs | 69 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ed53169e795c..c859a8984bae 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -18,6 +18,7 @@
 //
 // Stable since Rust 1.79.0.
 #![feature(inline_const)]
+#![feature(pointer_is_aligned)]
 //
 // Stable since Rust 1.81.0.
 #![feature(lint_reasons)]
diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 1c7d43771a37..7493b84b5474 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -2,6 +2,8 @@
 
 //! Traits for transmuting types.
 
+use core::mem::size_of;
+
 /// Types for which any bit pattern is valid.
 ///
 /// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
@@ -9,10 +11,74 @@
 ///
 /// It's okay for the type to have padding, as initializing those bytes has no effect.
 ///
+/// # Examples
+///
+/// ```
+/// use kernel::transmute::FromBytes;
+///
+/// fn test() -> Option<()> {
+///    let raw = [1, 2, 3, 4];
+///
+///    let result = u32::from_bytes(&raw)?;
+///
+///    #[cfg(target_endian = "little")]
+///    assert_eq!(*result, 0x4030201);
+///
+///    #[cfg(target_endian = "big")]
+///    assert_eq!(*result, 0x1020304);
+///
+///    Some(())
+/// }
+/// ```
+///
 /// # Safety
 ///
 /// All bit-patterns must be valid for this type. This type must not have interior mutability.
-pub unsafe trait FromBytes {}
+pub unsafe trait FromBytes {
+    /// Converts a slice of bytes to a reference to `Self`.
+    ///
+    /// When the reference is properly aligned and the size of slice is equal to that of `T`
+    /// and is different from zero.
+    ///
+    /// In another case, it will return `None`.
+    #[allow(clippy::incompatible_msrv)]
+    fn from_bytes(bytes: &[u8]) -> Option<&Self>
+    where
+        Self: Sized,
+    {
+        let slice_ptr = bytes.as_ptr().cast::<Self>();
+        let size = size_of::<Self>();
+        if bytes.len() == size && slice_ptr.is_aligned() {
+            // SAFETY: Checking for size and alignment ensure
+            // that the conversion to a type is valid
+            unsafe { Some(&*slice_ptr) }
+        } else {
+            None
+        }
+    }
+
+    /// Converts a mutable slice of bytes to a reference to `Self`
+    ///
+    /// When the reference is properly aligned and the size of slice
+    /// is equal to that of `T`and is different from zero.
+    ///
+    /// In another case, it will return `None`.
+    #[allow(clippy::incompatible_msrv)]
+    fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
+    where
+        Self: AsBytes + Sized,
+    {
+        let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
+        let size = size_of::<Self>();
+        if bytes.len() == size && slice_ptr.is_aligned() {
+            // SAFETY: Checking for size and alignment ensure
+            // that the conversion to a type is valid
+            unsafe { Some(&mut *slice_ptr) }
+        } else {
+            None
+        }
+    }
+}
 
 macro_rules! impl_frombytes {
     ($($({$($generics:tt)*})? $t:ty, )*) => {
@@ -28,7 +94,6 @@ macro_rules! impl_frombytes {
 
     // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
     // patterns are also acceptable for arrays of that type.
-    {<T: FromBytes>} [T],
     {<T: FromBytes, const N: usize>} [T; N],
 }
 
-- 
2.50.1

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Alice Ryhl]

On Sun, Aug 24, 2025 at 06:31:33PM -0300, Christian S. Lima wrote:
> The two methods added take a slice of bytes and return those bytes in
> a specific type. These methods are useful when we need to transform
> the stream of bytes into specific type.
> 
> Since the `is_aligned` method for pointer types has been stabilized in
> `1.79` version and is being used in this patch. I'm enabling the
> feature. In this case, using this method is useful to check the
> alignment and avoid a giant boilerplate, such as `(foo.as_ptr() as
> usize) % core::mem::align_of::<T>() == 0`.
> 
> Even enabling in `rust/kernel/lib.rs` when compiling with `make LLVM=1
> CLIPPY=1` a warning is issued, so in order to compile, it was used
> locally the `#[allow(clippy::incompatible_msrv)]`.
> 
> Link: https://github.com/Rust-for-Linux/linux/issues/1119
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Christian S. Lima <christiansantoslima21@gmail.com>

With my comments addressed:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> -pub unsafe trait FromBytes {}
> +pub unsafe trait FromBytes {
> +    /// Converts a slice of bytes to a reference to `Self`.
> +    ///
> +    /// When the reference is properly aligned and the size of slice is equal to that of `T`
> +    /// and is different from zero.
> +    ///
> +    /// In another case, it will return `None`.
> +    #[allow(clippy::incompatible_msrv)]

Does the warning appear on all configurations? If so, this #[allow]
should be an #[expect].

>  macro_rules! impl_frombytes {
>      ($($({$($generics:tt)*})? $t:ty, )*) => {
> @@ -28,7 +94,6 @@ macro_rules! impl_frombytes {
>  
>      // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
>      // patterns are also acceptable for arrays of that type.
> -    {<T: FromBytes>} [T],

Why is this impl removed? I would think that with the Self: Sized
bounds, you don't need to remove it anymore.

Alice

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Alexandre Courbot]

On Mon Aug 25, 2025 at 6:31 AM JST, Christian S. Lima wrote:
> The two methods added take a slice of bytes and return those bytes in
> a specific type. These methods are useful when we need to transform
> the stream of bytes into specific type.
>
> Since the `is_aligned` method for pointer types has been stabilized in
> `1.79` version and is being used in this patch. I'm enabling the
> feature. In this case, using this method is useful to check the
> alignment and avoid a giant boilerplate, such as `(foo.as_ptr() as
> usize) % core::mem::align_of::<T>() == 0`.
>
> Even enabling in `rust/kernel/lib.rs` when compiling with `make LLVM=1
> CLIPPY=1` a warning is issued, so in order to compile, it was used
> locally the `#[allow(clippy::incompatible_msrv)]`.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1119
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Christian S. Lima <christiansantoslima21@gmail.com>

Successfully tested with nova-core, thanks! This is an essential support
piece for the next steps in bringing up our driver.

Tested-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

If that works with everybody, I would like to take this into the Nova
tree (after a few more days of review) as we will be using it this
cycle. Miguel, is that ok with you?

Minor comments below, which we can address while merging so no need to
send a new version just for these.

> ---
> Changes in v2:
> - Rollback the implementation for the macro in the repository and implement
>   methods in trait
> - Link to v2: https://lore.kernel.org/rust-for-linux/20241012070121.110481-1-christiansantoslima21@gmail.com/
>
> Changes in v3:
> - Fix grammar errors
> - Remove repeated tests
> - Fix alignment errors
> - Fix tests not building
> - Link to v3: https://lore.kernel.org/rust-for-linux/20241109055442.85190-1-christiansantoslima21@gmail.com/
>
> Changes in v4:
> - Removed core::simd::ToBytes
> - Changed trait and methods to safe Add
> - Result<&Self, Error> in order to make safe methods
> - Link to v4: https://lore.kernel.org/rust-for-linux/20250314034910.134463-1-christiansantoslima21@gmail.com/
>
> Changes in v5:
> - Changed from Result to Option
> - Removed commentaries
> - Returned trait impl to unsafe
> - Link to v5: https://lore.kernel.org/rust-for-linux/20250320014041.101470-1-christiansantoslima21@gmail.com/
>
> Changes in v6:
> - Add endianess check to doc test and use match to check
> success case
> - Reformulated safety comments
> - Link to v6: https://lore.kernel.org/rust-for-linux/20250330234039.29814-1-christiansantoslima21@gmail.com/
>
> Changes in v7:
> - Add alignment check
> - Link to v7: https://lore.kernel.org/rust-for-linux/20250615072042.133290-1-christiansantoslima21@gmail.com/
>
> Changes in v8:
> - Add the new FromBytesSized trait
> - Change the implementation of FromBytes trait methods
> - Move the cast to pointer earlier and use `is_aligned()` instead manual
> alignment check
> - Link to v8: https://lore.kernel.org/rust-for-linux/20250624042802.105623-1-christiansantoslima21@gmail.com/
>
> Changes in v9:
> - Improve code comments and remove confusing parts.
> - Add a build_assert in the conversion of type `[T]` to check for elements
> inside the slice.
> - Count the elements in the `[T]` conversion instead of using byte
> count.
> - Link to v9: https://lore.kernel.org/rust-for-linux/20250811213851.65644-1-christiansantoslima21@gmail.com/#t
>
> Changes in v10:
> - Remove `FromBytesSized` trait
> - Remove implementation for slice types
> - Fix doctest not compiling because `?` operator outside a function
> that return `Option<()>`
> - Make `FromBytes` trait depend on `Sized`
> - Add `is_aligned` as feature
> ---
>  rust/kernel/lib.rs       |  1 +
>  rust/kernel/transmute.rs | 69 ++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ed53169e795c..c859a8984bae 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -18,6 +18,7 @@
>  //
>  // Stable since Rust 1.79.0.
>  #![feature(inline_const)]
> +#![feature(pointer_is_aligned)]
>  //
>  // Stable since Rust 1.81.0.
>  #![feature(lint_reasons)]
> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> index 1c7d43771a37..7493b84b5474 100644
> --- a/rust/kernel/transmute.rs
> +++ b/rust/kernel/transmute.rs
> @@ -2,6 +2,8 @@
>  
>  //! Traits for transmuting types.
>  
> +use core::mem::size_of;
> +
>  /// Types for which any bit pattern is valid.
>  ///
>  /// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
> @@ -9,10 +11,74 @@
>  ///
>  /// It's okay for the type to have padding, as initializing those bytes has no effect.
>  ///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::transmute::FromBytes;
> +///
> +/// fn test() -> Option<()> {
> +///    let raw = [1, 2, 3, 4];
> +///
> +///    let result = u32::from_bytes(&raw)?;
> +///
> +///    #[cfg(target_endian = "little")]
> +///    assert_eq!(*result, 0x4030201);
> +///
> +///    #[cfg(target_endian = "big")]
> +///    assert_eq!(*result, 0x1020304);
> +///
> +///    Some(())
> +/// }
> +/// ```
> +///
>  /// # Safety
>  ///
>  /// All bit-patterns must be valid for this type. This type must not have interior mutability.
> -pub unsafe trait FromBytes {}
> +pub unsafe trait FromBytes {
> +    /// Converts a slice of bytes to a reference to `Self`.
> +    ///
> +    /// When the reference is properly aligned and the size of slice is equal to that of `T`
> +    /// and is different from zero.
> +    ///
> +    /// In another case, it will return `None`.

I'd replace "When" by "If" and "In another case" with "Otherwise" to
sound more natural.

> +    #[allow(clippy::incompatible_msrv)]
> +    fn from_bytes(bytes: &[u8]) -> Option<&Self>
> +    where
> +        Self: Sized,
> +    {
> +        let slice_ptr = bytes.as_ptr().cast::<Self>();
> +        let size = size_of::<Self>();
> +        if bytes.len() == size && slice_ptr.is_aligned() {
> +            // SAFETY: Checking for size and alignment ensure
> +            // that the conversion to a type is valid
> +            unsafe { Some(&*slice_ptr) }
> +        } else {
> +            None
> +        }
> +    }
> +
> +    /// Converts a mutable slice of bytes to a reference to `Self`
> +    ///
> +    /// When the reference is properly aligned and the size of slice
> +    /// is equal to that of `T`and is different from zero.
> +    ///
> +    /// In another case, it will return `None`.
> +    #[allow(clippy::incompatible_msrv)]
> +    fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
> +    where
> +        Self: AsBytes + Sized,
> +    {
> +        let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
> +        let size = size_of::<Self>();
> +        if bytes.len() == size && slice_ptr.is_aligned() {
> +            // SAFETY: Checking for size and alignment ensure
> +            // that the conversion to a type is valid
> +            unsafe { Some(&mut *slice_ptr) }
> +        } else {
> +            None
> +        }
> +    }
> +}
>  
>  macro_rules! impl_frombytes {
>      ($($({$($generics:tt)*})? $t:ty, )*) => {
> @@ -28,7 +94,6 @@ macro_rules! impl_frombytes {
>  
>      // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
>      // patterns are also acceptable for arrays of that type.
> -    {<T: FromBytes>} [T],

As Alice pointed out, there should be no need to remove this - I have
kept it and things build just fine.

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Miguel Ojeda]

On Sun, Aug 24, 2025 at 11:31 PM Christian S. Lima
<christiansantoslima21@gmail.com> wrote:
>
> Since the `is_aligned` method for pointer types has been stabilized in
> `1.79` version and is being used in this patch. I'm enabling the
> feature.

Period -> comma? No need to backquote version numbers.

> Even enabling in `rust/kernel/lib.rs` when compiling with `make LLVM=1
> CLIPPY=1` a warning is issued, so in order to compile, it was used
> locally the `#[allow(clippy::incompatible_msrv)]`.

I would perhaps try to reword this a bit.

> Link: https://github.com/Rust-for-Linux/linux/issues/1119
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>

Please add a link to the original suggestion if possible. If it is the
link above, then the tags should be in the opposite order.

Did Benno and Alexandre both suggest it?

> +/// fn test() -> Option<()> {
> +///    let raw = [1, 2, 3, 4];
> +///
> +///    let result = u32::from_bytes(&raw)?;
> +///
> +///    #[cfg(target_endian = "little")]
> +///    assert_eq!(*result, 0x4030201);
> +///
> +///    #[cfg(target_endian = "big")]
> +///    assert_eq!(*result, 0x1020304);
> +///
> +///    Some(())
> +/// }

Should the function be called? Otherwise, we are only build-testing this.

Should we just remove the function and to it directly at the top-level?

> +    /// Converts a slice of bytes to a reference to `Self`.

An intra-doc link may work here.

> +    /// In another case, it will return `None`.

Ditto (also elsewhere).

I would perhaps just say "Otherwise," to simplify.

> +    #[allow(clippy::incompatible_msrv)]

If this can be made more local, then please do so; otherwise, no big deal.

> +            // SAFETY: Checking for size and alignment ensure
> +            // that the conversion to a type is valid

Period at the end. I would probably say "Size and alignment were just
checked, thus ...".

> +    /// Converts a mutable slice of bytes to a reference to `Self`

Period at the end, intra-doc link if possible (ditto elsewhere).

> +    /// When the reference is properly aligned and the size of slice
> +    /// is equal to that of `T`and is different from zero.

Missing space after `T`.

These are all mostly nits that I sometimes fixed on-apply, for
Alexandre in case it helps, i.e. no need to send a new version if it
is getting applied already.

The actually-run-the-test is probably the most important one.

Thanks!

Cheers,
Miguel

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Miguel Ojeda]

On Mon, Aug 25, 2025 at 12:39 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> If that works with everybody, I would like to take this into the Nova
> tree (after a few more days of review) as we will be using it this
> cycle. Miguel, is that ok with you?

Sure! I am leaving a few notes in another reply of things I would have
probably fixed on apply, in case it helps.

Thanks!

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Christian]

Hi, Alice.

> Does the warning appear on all configurations? If so, this #[allow]
> should be an #[expect].

If clippy is disabled, nothing is reported.

> >  macro_rules! impl_frombytes {
> >      ($($({$($generics:tt)*})? $t:ty, )*) => {
> > @@ -28,7 +94,6 @@ macro_rules! impl_frombytes {
> >
> >      // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
> >      // patterns are also acceptable for arrays of that type.
> > -    {<T: FromBytes>} [T],
>
> Why is this impl removed? I would think that with the Self: Sized
> bounds, you don't need to remove it anymore.

I interpreted too literally the idea of removing the implementation of slices.

Thanks,
Christian

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Christian]

Hi, Miguel.


> > Link: https://github.com/Rust-for-Linux/linux/issues/1119
> > Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Please add a link to the original suggestion if possible. If it is the
> link above, then the tags should be in the opposite order.
>
> Did Benno and Alexandre both suggest it?

Benno suggested the link to github in the issue there. Alexandre
helped in most of the design. It was a mistake by my part not to
specify that. Even if it's good, put you too as suggested-by, because
the idea of using `is_aligned` was yours. Maybe Alexandre can put it
there?

If I understand correctly, should be:

Suggested-by: Benno Lossin <benno.lossin@proton.me>,
Link: https://github.com/Rust-for-Linux/linux/issues/1119

Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Link: https://lore.kernel.org/rust-for-linux/DC5INTQKY0EX.1T4HD6OU8C4PI@nvidia.com/

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/CANiq72mnxRquFmjoJemb=3LSq+ZdUfs9J+HXTwM6AavprsVNUg@mail.gmail.com/

> > +/// fn test() -> Option<()> {
> > +///    let raw = [1, 2, 3, 4];
> > +///
> > +///    let result = u32::from_bytes(&raw)?;
> > +///
> > +///    #[cfg(target_endian = "little")]
> > +///    assert_eq!(*result, 0x4030201);
> > +///
> > +///    #[cfg(target_endian = "big")]
> > +///    assert_eq!(*result, 0x1020304);
> > +///
> > +///    Some(())
> > +/// }
>
> Should the function be called? Otherwise, we are only build-testing this.
>
> Should we just remove the function and to it directly at the top-level?

I think just calling `test()` is good, but it's an aesthetic preference.

Thanks,
Christian

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Alexandre Courbot]

On Tue Aug 26, 2025 at 4:51 AM JST, Christian wrote:
> Hi, Miguel.
>
>
>> > Link: https://github.com/Rust-for-Linux/linux/issues/1119
>> > Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
>>
>> Please add a link to the original suggestion if possible. If it is the
>> link above, then the tags should be in the opposite order.
>>
>> Did Benno and Alexandre both suggest it?
>
> Benno suggested the link to github in the issue there. Alexandre
> helped in most of the design. It was a mistake by my part not to
> specify that. Even if it's good, put you too as suggested-by, because
> the idea of using `is_aligned` was yours. Maybe Alexandre can put it
> there?
>
> If I understand correctly, should be:
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>,
> Link: https://github.com/Rust-for-Linux/linux/issues/1119
>
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Link: https://lore.kernel.org/rust-for-linux/DC5INTQKY0EX.1T4HD6OU8C4PI@nvidia.com/
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72mnxRquFmjoJemb=3LSq+ZdUfs9J+HXTwM6AavprsVNUg@mail.gmail.com/

Unless Miguel disagrees, I'll just keep Benno's entry as what Miguel and
I did was more part of the regular review process IMHO.

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Alexandre Courbot]

Thanks for the feedback Miguel! For the record, here are the changes I am
planning on applying on top of this version:

On Mon Aug 25, 2025 at 11:51 PM JST, Miguel Ojeda wrote:
> On Sun, Aug 24, 2025 at 11:31 PM Christian S. Lima
> <christiansantoslima21@gmail.com> wrote:
>>
>> Since the `is_aligned` method for pointer types has been stabilized in
>> `1.79` version and is being used in this patch. I'm enabling the
>> feature.
>
> Period -> comma? No need to backquote version numbers.
>
>> Even enabling in `rust/kernel/lib.rs` when compiling with `make LLVM=1
>> CLIPPY=1` a warning is issued, so in order to compile, it was used
>> locally the `#[allow(clippy::incompatible_msrv)]`.
>
> I would perhaps try to reword this a bit.

Reworded this to:

Also add `#[allow(clippy::incompatible_msrv)]` where needed until the
MSRV is updated to `1.79`.

>
>> Link: https://github.com/Rust-for-Linux/linux/issues/1119
>> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Please add a link to the original suggestion if possible. If it is the
> link above, then the tags should be in the opposite order.
>
> Did Benno and Alexandre both suggest it?
>
>> +/// fn test() -> Option<()> {
>> +///    let raw = [1, 2, 3, 4];
>> +///
>> +///    let result = u32::from_bytes(&raw)?;
>> +///
>> +///    #[cfg(target_endian = "little")]
>> +///    assert_eq!(*result, 0x4030201);
>> +///
>> +///    #[cfg(target_endian = "big")]
>> +///    assert_eq!(*result, 0x1020304);
>> +///
>> +///    Some(())
>> +/// }
>
> Should the function be called? Otherwise, we are only build-testing this.
>
> Should we just remove the function and to it directly at the top-level?

Changed this to call the function and hide the unneeded parts:

--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -16,19 +16,20 @@
 /// ```
 /// use kernel::transmute::FromBytes;
 ///
-/// fn test() -> Option<()> {
-///    let raw = [1, 2, 3, 4];
+/// # fn test() -> Option<()> {
+/// let raw = [1, 2, 3, 4];
 ///
-///    let result = u32::from_bytes(&raw)?;
+/// let result = u32::from_bytes(&raw)?;
 ///
-///    #[cfg(target_endian = "little")]
-///    assert_eq!(*result, 0x4030201);
+/// #[cfg(target_endian = "little")]
+/// assert_eq!(*result, 0x4030201);
 ///
-///    #[cfg(target_endian = "big")]
-///    assert_eq!(*result, 0x1020304);
+/// #[cfg(target_endian = "big")]
+/// assert_eq!(*result, 0x1020304);
 ///
-///    Some(())
-/// }
+/// # Some(()) }
+/// # test().ok_or(EINVAL)?;
+/// # Ok::<(), Error>(())

>
>> +    /// Converts a slice of bytes to a reference to `Self`.
>
> An intra-doc link may work here.

Wasn't sure what a link to `Self` might link to in a trait declaration, so left
this one as-is. :)

>
>> +    /// In another case, it will return `None`.
>
> Ditto (also elsewhere).
>
> I would perhaps just say "Otherwise," to simplify.
>
>> +    #[allow(clippy::incompatible_msrv)]
>
> If this can be made more local, then please do so; otherwise, no big deal.

Moved this closer to where it is needed:

--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -41,13 +41,14 @@ pub unsafe trait FromBytes {
     /// and is different from zero.
     ///
     /// In another case, it will return `None`.
-    #[allow(clippy::incompatible_msrv)]
     fn from_bytes(bytes: &[u8]) -> Option<&Self>
     where
         Self: Sized,
     {
         let slice_ptr = bytes.as_ptr().cast::<Self>();
         let size = size_of::<Self>();
+
+        #[allow(clippy::incompatible_msrv)]
         if bytes.len() == size && slice_ptr.is_aligned() {
             // SAFETY: Checking for size and alignment ensure
             // that the conversion to a type is valid
@@ -63,13 +64,14 @@ fn from_bytes(bytes: &[u8]) -> Option<&Self>
     /// is equal to that of `T`and is different from zero.
     ///
     /// In another case, it will return `None`.
-    #[allow(clippy::incompatible_msrv)]
     fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
     where
         Self: AsBytes + Sized,
     {
         let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
         let size = size_of::<Self>();
+
+        #[allow(clippy::incompatible_msrv)]
         if bytes.len() == size && slice_ptr.is_aligned() {
             // SAFETY: Checking for size and alignment ensure


>
>> +            // SAFETY: Checking for size and alignment ensure
>> +            // that the conversion to a type is valid
>
> Period at the end. I would probably say "Size and alignment were just
> checked, thus ...".
>
>> +    /// Converts a mutable slice of bytes to a reference to `Self`
>
> Period at the end, intra-doc link if possible (ditto elsewhere).

Changed the doc/comments as follows:

@@ -38,10 +38,10 @@
 pub unsafe trait FromBytes {
     /// Converts a slice of bytes to a reference to `Self`.
     ///
-    /// When the reference is properly aligned and the size of slice is equal to that of `T`
-    /// and is different from zero.
+    /// Succeeds if the reference is properly aligned, and the size of `bytes` is equal to that of
+    /// `T` and different from zero.
     ///
-    /// In another case, it will return `None`.
+    /// Otherwise, returns [`None`].
     fn from_bytes(bytes: &[u8]) -> Option<&Self>
     where
         Self: Sized,
@@ -51,20 +51,19 @@ fn from_bytes(bytes: &[u8]) -> Option<&Self>

         #[allow(clippy::incompatible_msrv)]
         if bytes.len() == size && slice_ptr.is_aligned() {
-            // SAFETY: Checking for size and alignment ensure
-            // that the conversion to a type is valid
+            // SAFETY: Size and alignment were just checked.
             unsafe { Some(&*slice_ptr) }
         } else {
             None
         }
     }

-    /// Converts a mutable slice of bytes to a reference to `Self`
+    /// Converts a mutable slice of bytes to a reference to `Self`.
     ///
-    /// When the reference is properly aligned and the size of slice
-    /// is equal to that of `T`and is different from zero.
+    /// Succeeds if the reference is properly aligned, and the size of `bytes` is equal to that of
+    /// `T` and different from zero.
     ///
-    /// In another case, it will return `None`.
+    /// Otherwise, returns [`None`].
     fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
     where
         Self: AsBytes + Sized,
@@ -74,8 +73,7 @@ fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>

         #[allow(clippy::incompatible_msrv)]
         if bytes.len() == size && slice_ptr.is_aligned() {
-            // SAFETY: Checking for size and alignment ensure
-            // that the conversion to a type is valid
+            // SAFETY: Size and alignment were just checked.
             unsafe { Some(&mut *slice_ptr) }
         } else {
             None

Also, as suggested by Alice I am re-enabling the implementation on slices:

--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -94,6 +94,7 @@ macro_rules! impl_frombytes {

     // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
     // patterns are also acceptable for arrays of that type.
+    {<T: FromBytes>} [T],
     {<T: FromBytes, const N: usize>} [T; N],
 }

Unless someone says something by then, I plan on pushing this to nova-next
tomorrow.

Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait

[Alexandre Courbot]

On Thu Aug 28, 2025 at 3:26 PM JST, Alexandre Courbot wrote:
> Unless someone says something by then, I plan on pushing this to nova-next
> tomorrow.

Pushed into nova-next! Thanks everyone.