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

["Alexandre Courbot" <acourbot@nvidia.com>]

Hi Christian,

Thanks for this version! (and sorry for being late to come back to it) I
think it looks pretty good and  could adapt the nova-core code to use it
without any issue. A few comments inline:

On Tue Jun 24, 2025 at 1:28 PM 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.
>
> The `FromBytesSized` trait was added to make it easier to implement other
> user defined types within the codebase. With the current implementation,
> there's no way to interact without implementing `from_bytes` and
> `from_mut_bytes`for every new type, and this would end up generating a lot
> of duplicate code. By using FromBytesSized as a proxy trait, we can avoid
> this without generating a direct dependecy. If necessary, the user can

nit: s/dependecy/dependency.

<snip>
> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> index 1c7d43771a37..832c65a1239c 100644
> --- a/rust/kernel/transmute.rs
> +++ b/rust/kernel/transmute.rs
> @@ -9,27 +9,115 @@
>  ///
>  /// It's okay for the type to have padding, as initializing those bytes has no effect.
>  ///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::transmute::FromBytes;
> +///
> +/// let foo = [1, 2, 3, 4];
> +///
> +/// let result = u32::from_bytes(&foo)?;
> +///
> +/// #[cfg(target_endian = "little")]
> +/// assert_eq!(*result, 0x4030201);
> +///
> +/// #[cfg(target_endian = "big")]
> +/// assert_eq!(*result, 0x1020304);
> +/// ```
> +///
>  /// # 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 possible.

Let's elaborate on when it is "possible", i.e. the reference is properly
aligned, and the size of the slice is equal to that of `T`. Let's also
clarify that `None` is returned in other cases.

> +    fn from_bytes(bytes: &[u8]) -> Option<&Self>;
> +
> +    /// Converts a mutable slice of bytes to a reference to `Self` when possible.

Same here.

> +    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>

`from_bytes_mut` sounds like a more idiomatic name for this method.

> +    where
> +        Self: AsBytes;
> +}

Note that `samples/rust/rust_dma.rs` will fail to compile due to this
change - you must make it derive `FromBytesSized` instead. There may be
other implementors of `FromBytes` so please make sure to track and
update them to avoid breaking the build.

nova-next also adds new implementations of `FromBytes`, and since they
are not in mainline yet this will make it harder to adapt them... I see
two possible solutions if we want this for the next cycle:

- Take this patch into nova-next and update `FromBytes` implementations
  in lockstep,
- Add temporary default implementations for `from_bytes` and
  `from_mut_bytes` that simply return `None` so current implementors
  keep building.

>  
> -macro_rules! impl_frombytes {
> +/// Just a proxy trait for FromBytes, if you need an implementation for your type use this instead.

Let's explain the reason for having this type (provide an
auto-implementation of `FromBytes`'s methods for all sized types),
otherwise it may be confusing.

> +///
> +/// # Safety
> +///
> +/// All bit-patterns must be valid for this type. This type must not have interior mutability.
> +pub unsafe trait FromBytesSized: Sized {}
> +
> +macro_rules! impl_frombytessized {
>      ($($({$($generics:tt)*})? $t:ty, )*) => {
>          // SAFETY: Safety comments written in the macro invocation.
> -        $(unsafe impl$($($generics)*)? FromBytes for $t {})*
> +        $(unsafe impl$($($generics)*)? FromBytesSized for $t {})*
>      };
>  }
>  
> -impl_frombytes! {
> +impl_frombytessized! {
>      // SAFETY: All bit patterns are acceptable values of the types below.
>      u8, u16, u32, u64, usize,
>      i8, i16, i32, i64, isize,
>  
>      // 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],
> +    {<T: FromBytesSized, const N: usize>} [T; N],
> +}
> +
> +// SAFETY: All bit patterns are acceptable values of the types and in array case if all bit patterns

SAFETY: The `FromBytesSized` implementation guarantees that all bit
patterns ...

> +// are acceptable for individual values in an array, then all bit patterns are also acceptable
> +// for arrays of that type.

I don't think this implementation takes care of arrays?