linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8] rust: transmute: Add methods for FromBytes trait
@ 2025-06-24  4:28 Christian S. Lima
  2025-06-25  2:39 ` kernel test robot
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Christian S. Lima @ 2025-06-24  4:28 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
	~lkcamp/patches, richard120310

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
simply implement `FromBytes`if needed. For more context please, check the
[1] and [2].

[1] https://lore.kernel.org/rust-for-linux/DANSZ6Q476EC.3GY00K717QVUL@nvidia.com/
[2] https://lore.kernel.org/rust-for-linux/DAOESYD6F287.3U3M64X0S1WN5@nvidia.com/

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
---
 rust/kernel/transmute.rs | 100 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 6 deletions(-)

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.
+    fn from_bytes(bytes: &[u8]) -> Option<&Self>;
+
+    /// Converts a mutable slice of bytes to a reference to `Self` when possible.
+    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
+    where
+        Self: AsBytes;
+}
 
-macro_rules! impl_frombytes {
+/// Just a proxy trait for FromBytes, if you need an implementation for your type use this instead.
+///
+/// # 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
+// are acceptable for individual values in an array, then all bit patterns are also acceptable
+// for arrays of that type.
+unsafe impl<T> FromBytes for T
+where
+    T: FromBytesSized,
+{
+    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
+        let slice_ptr = bytes.as_ptr().cast::<T>();
+        if bytes.len() == ::core::mem::size_of::<T>() && slice_ptr.is_aligned() {
+            // SAFETY: Since the code checks the size and alignment, the slice is valid.
+            unsafe { Some(&*slice_ptr) }
+        } else {
+            None
+        }
+    }
+
+    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
+    where
+        Self: AsBytes,
+    {
+        let slice_ptr = bytes.as_mut_ptr().cast::<T>();
+        if bytes.len() == ::core::mem::size_of::<T>() && slice_ptr.is_aligned() {
+            // SAFETY: Since the code checks the size and alignment, the slice is valid.
+            unsafe { Some(&mut *slice_ptr) }
+        } else {
+            None
+        }
+    }
+}
+
+// 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.
+unsafe impl<T: FromBytes> FromBytes for [T] {
+    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
+        let slice_ptr = bytes.as_ptr().cast::<T>();
+        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
+            // SAFETY: Since the code checks the size and alignment, the slice is valid.
+            unsafe { Some(::core::slice::from_raw_parts(slice_ptr, bytes.len())) }
+        } else {
+            None
+        }
+    }
+
+    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
+    where
+        Self: AsBytes,
+    {
+        let slice_ptr = bytes.as_mut_ptr().cast::<T>();
+        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
+            // SAFETY: Since the code checks the size and alignment, the slice is valid.
+            unsafe { Some(::core::slice::from_raw_parts_mut(slice_ptr, bytes.len())) }
+        } else {
+            None
+        }
+    }
 }
 
 /// Types that can be viewed as an immutable slice of initialized bytes.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-06-24  4:28 [PATCH v8] rust: transmute: Add methods for FromBytes trait Christian S. Lima
@ 2025-06-25  2:39 ` kernel test robot
  2025-07-07  5:14 ` Alexandre Courbot
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-06-25  2:39 UTC (permalink / raw)
  To: Christian S. Lima, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel, ~lkcamp/patches, richard120310
  Cc: llvm, oe-kbuild-all

Hi Christian,

kernel test robot noticed the following build errors:

[auto build test ERROR on rust/rust-next]
[also build test ERROR on linus/master v6.16-rc3 next-20250624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-S-Lima/rust-transmute-Add-methods-for-FromBytes-trait/20250624-123012
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20250624042802.105623-1-christiansantoslima21%40gmail.com
patch subject: [PATCH v8] rust: transmute: Add methods for FromBytes trait
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250625/202506251052.onsnGcvR-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250625/202506251052.onsnGcvR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506251052.onsnGcvR-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0658]: use of unstable library feature 'pointer_is_aligned'
   --> rust/kernel/transmute.rs:74:68
   |
   74 |         if bytes.len() == ::core::mem::size_of::<T>() && slice_ptr.is_aligned() {
   |                                                                    ^^^^^^^^^^
   |
   = note: see issue #96284 <https://github.com/rust-lang/rust/issues/96284> for more information
   = help: add `#![feature(pointer_is_aligned)]` to the crate attributes to enable
   = note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
--
>> error[E0658]: use of unstable library feature 'pointer_is_aligned'
   --> rust/kernel/transmute.rs:87:68
   |
   87 |         if bytes.len() == ::core::mem::size_of::<T>() && slice_ptr.is_aligned() {
   |                                                                    ^^^^^^^^^^
   |
   = note: see issue #96284 <https://github.com/rust-lang/rust/issues/96284> for more information
   = help: add `#![feature(pointer_is_aligned)]` to the crate attributes to enable
   = note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
--
>> error[E0658]: use of unstable library feature 'pointer_is_aligned'
   --> rust/kernel/transmute.rs:101:72
   |
   101 |         if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
   |                                                                        ^^^^^^^^^^
   |
   = note: see issue #96284 <https://github.com/rust-lang/rust/issues/96284> for more information
   = help: add `#![feature(pointer_is_aligned)]` to the crate attributes to enable
   = note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
--
>> error[E0658]: use of unstable library feature 'pointer_is_aligned'
   --> rust/kernel/transmute.rs:114:72
   |
   114 |         if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
   |                                                                        ^^^^^^^^^^
   |
   = note: see issue #96284 <https://github.com/rust-lang/rust/issues/96284> for more information
   = help: add `#![feature(pointer_is_aligned)]` to the crate attributes to enable
   = note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-06-24  4:28 [PATCH v8] rust: transmute: Add methods for FromBytes trait Christian S. Lima
  2025-06-25  2:39 ` kernel test robot
@ 2025-07-07  5:14 ` Alexandre Courbot
  2025-07-14 22:16   ` Christian
  2025-07-27  1:37 ` Kane York
  2025-08-01 12:26 ` Alexandre Courbot
  3 siblings, 1 reply; 13+ messages in thread
From: Alexandre Courbot @ 2025-07-07  5:14 UTC (permalink / raw)
  To: Christian S. Lima, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel, ~lkcamp/patches, richard120310

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?


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-07  5:14 ` Alexandre Courbot
@ 2025-07-14 22:16   ` Christian
  2025-07-22 14:06     ` Alexandre Courbot
  0 siblings, 1 reply; 13+ messages in thread
From: Christian @ 2025-07-14 22:16 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
	~lkcamp/patches, richard120310

Hi, Alexandre. No problem, take your time.

> 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.

I see, thanks for a better explanation, I'll include it in the next patch.

> > +    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
>
> `from_bytes_mut` sounds like a more idiomatic name for this method.

It is done this way to match the zero copy API. [1]

> > +    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.

In this case, if we don't include `AsBytes` the user can add padding
bytes in the slice. [2]

> 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.

I think the first option is better considering the current state.

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

Arrays are sized types implemented in `FromBytesSized`, I believe in
this case arrays are implemented too.

[1] https://lore.kernel.org/rust-for-linux/CAGSQo03teSsTa84Mx=4SFRAfa2=irBwgXXoh5eB_Q0RoixrF6Q@mail.gmail.com/
[2] https://lore.kernel.org/rust-for-linux/D8FXFGYZTEXT.24UM4V3HZ5MWH@proton.me/

Thanks,
Christian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-14 22:16   ` Christian
@ 2025-07-22 14:06     ` Alexandre Courbot
  2025-07-25 18:37       ` Christian
  0 siblings, 1 reply; 13+ messages in thread
From: Alexandre Courbot @ 2025-07-22 14:06 UTC (permalink / raw)
  To: Christian
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
	~lkcamp/patches, richard120310

On Tue Jul 15, 2025 at 7:16 AM JST, Christian wrote:
> Hi, Alexandre. No problem, take your time.
>
>> 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.
>
> I see, thanks for a better explanation, I'll include it in the next patch.
>
>> > +    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
>>
>> `from_bytes_mut` sounds like a more idiomatic name for this method.
>
> It is done this way to match the zero copy API. [1]

Mmm I have checked the zerocopy doc and could not find any instance of
either `from_bytes_mut` or `from_mut_bytes`...

>
>> > +    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.
>
> In this case, if we don't include `AsBytes` the user can add padding
> bytes in the slice. [2]

To clarify, I am not complaining about the `Self: AsBytes` requirement,
I am just pointing out that you will need to update the sample so it
keeps building. :)

>
>> 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.
>
> I think the first option is better considering the current state.

Sounds good to me as well!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-22 14:06     ` Alexandre Courbot
@ 2025-07-25 18:37       ` Christian
  2025-07-26  2:57         ` Alexandre Courbot
  0 siblings, 1 reply; 13+ messages in thread
From: Christian @ 2025-07-25 18:37 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

Hi, Alexandre.

> To clarify, I am not complaining about the `Self: AsBytes` requirement,
> I am just pointing out that you will need to update the sample so it
> keeps building. :)

I see, I was just justifying the decision, sorry if it sounded a bit
aggressive. English isn't my primary language, and mistakes happen.
Now, about the patch. Should I submit a patch to fix the broken build?

> Sounds good to me as well!

Should I submit this patch to nova-next?

Thanks,
Christian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-25 18:37       ` Christian
@ 2025-07-26  2:57         ` Alexandre Courbot
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2025-07-26  2:57 UTC (permalink / raw)
  To: Christian
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

Hi Christian,

On Sat Jul 26, 2025 at 3:37 AM JST, Christian wrote:
> Hi, Alexandre.
>
>> To clarify, I am not complaining about the `Self: AsBytes` requirement,
>> I am just pointing out that you will need to update the sample so it
>> keeps building. :)
>
> I see, I was just justifying the decision, sorry if it sounded a bit
> aggressive. English isn't my primary language, and mistakes happen.
> Now, about the patch. Should I submit a patch to fix the broken build?

Your English sounded perfectly fine as far as I can tell (not a native
speaker myself :)).

Yes, please send a v9 addressing the remaining issues we had on v8.
Hopefully we can gather a few Reviewed-by and merge it.

>
>> Sounds good to me as well!
>
> Should I submit this patch to nova-next?

Just sending it the same way you did so far will be fine, all the
relevant folks are watching.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-06-24  4:28 [PATCH v8] rust: transmute: Add methods for FromBytes trait Christian S. Lima
  2025-06-25  2:39 ` kernel test robot
  2025-07-07  5:14 ` Alexandre Courbot
@ 2025-07-27  1:37 ` Kane York
  2025-07-28 19:39   ` Christian
  2025-08-01 12:26 ` Alexandre Courbot
  3 siblings, 1 reply; 13+ messages in thread
From: Kane York @ 2025-07-27  1:37 UTC (permalink / raw)
  To: christiansantoslima21
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, dakr, gary, linux-kernel, ojeda, richard120310,
	rust-for-linux, tmgross, ~lkcamp/patches

On Tue, 24 Jun 2025 01:28:02 -0300, Christian S. Lima wrote:
> +/// let result = u32::from_bytes(&foo)?;
> +///
> +/// #[cfg(target_endian = "little")]
> +/// assert_eq!(*result, 0x4030201);
> +///
> +/// #[cfg(target_endian = "big")]
> +/// assert_eq!(*result, 0x1020304);

I might start using this as a great example of conditional compilation.

> +// 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.
> +unsafe impl<T: FromBytes> FromBytes for [T] {
> +    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
> +        let slice_ptr = bytes.as_ptr().cast::<T>();
> +        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
> +            // SAFETY: Since the code checks the size and alignment, the slice is valid.
> +            unsafe { Some(::core::slice::from_raw_parts(slice_ptr, bytes.len())) }

This is incorrect -- the second argument to slice::from_raw_parts is the
element count, not the byte count.

> +        } else {
> +            None
> +        }
> +    }
> +
> +    fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
> +    where
> +        Self: AsBytes,
> +    {
> +        let slice_ptr = bytes.as_mut_ptr().cast::<T>();
> +        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
> +            // SAFETY: Since the code checks the size and alignment, the slice is valid.
> +            unsafe { Some(::core::slice::from_raw_parts_mut(slice_ptr, bytes.len())) }

Same here.

> +        } else {
> +            None
> +        }
> +    }
>  }
>
>  /// Types that can be viewed as an immutable slice of initialized bytes.
> --
> 2.49.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-27  1:37 ` Kane York
@ 2025-07-28 19:39   ` Christian
  2025-07-28 21:21     ` K. York
  0 siblings, 1 reply; 13+ messages in thread
From: Christian @ 2025-07-28 19:39 UTC (permalink / raw)
  To: Kane York
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, dakr, gary, linux-kernel, ojeda, richard120310,
	rust-for-linux, tmgross, ~lkcamp/patches

Hi, Kane.

> > +// 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.
> > +unsafe impl<T: FromBytes> FromBytes for [T] {
> > +    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
> > +        let slice_ptr = bytes.as_ptr().cast::<T>();
> > +        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
> > +            // SAFETY: Since the code checks the size and alignment, the slice is valid.
> > +            unsafe { Some(::core::slice::from_raw_parts(slice_ptr, bytes.len())) }
>
> This is incorrect -- the second argument to slice::from_raw_parts is the
> element count, not the byte count.

I don't understand, did you mean that the safety comment should be
changed or the argument? If you can explain in more detail.

Thanks,
Christian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-07-28 19:39   ` Christian
@ 2025-07-28 21:21     ` K. York
  0 siblings, 0 replies; 13+ messages in thread
From: K. York @ 2025-07-28 21:21 UTC (permalink / raw)
  To: Christian
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, dakr, gary, linux-kernel, ojeda, richard120310,
	rust-for-linux, tmgross, ~lkcamp/patches

On Mon, Jul 28, 2025, 12:39 Christian <christiansantoslima21@gmail.com> wrote:
>
> Hi, Kane.
>
> > > +// 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.
> > > +unsafe impl<T: FromBytes> FromBytes for [T] {
> > > +    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
> > > +        let slice_ptr = bytes.as_ptr().cast::<T>();
> > > +        if bytes.len() % ::core::mem::size_of::<T>() == 0 && slice_ptr.is_aligned() {
> > > +            // SAFETY: Since the code checks the size and alignment, the slice is valid.
> > > +            unsafe { Some(::core::slice::from_raw_parts(slice_ptr, bytes.len())) }
> >
> > This is incorrect -- the second argument to slice::from_raw_parts is the
> > element count, not the byte count.
>
> I don't understand, did you mean that the safety comment should be
> changed or the argument? If you can explain in more detail.

The code is wrong, and the comments should be changed to match.
The documentation for the function says:

`pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize)
-> &'a [T]`

> Forms a slice from a pointer and a length.

> The len argument is the number of **elements**, not the number of bytes.

The code is wrong for any type that is not 1 byte in size. The len
argument should
be `bytes.len() / ::core::mem::size_of::<T>()`.

The code will also panic the kernel for any type that is 0 bytes in
size in the % above.
I'd probably choose a build assert (`const { if size == 0 {
panic!(...) } }`) for this.

>
> Thanks,
> Christian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-06-24  4:28 [PATCH v8] rust: transmute: Add methods for FromBytes trait Christian S. Lima
                   ` (2 preceding siblings ...)
  2025-07-27  1:37 ` Kane York
@ 2025-08-01 12:26 ` Alexandre Courbot
  2025-08-01 15:35   ` Miguel Ojeda
  2025-08-01 17:47   ` Christian
  3 siblings, 2 replies; 13+ messages in thread
From: Alexandre Courbot @ 2025-08-01 12:26 UTC (permalink / raw)
  To: Christian S. Lima, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel, ~lkcamp/patches, richard120310

Hi Christian,

Any plan for a v9 soon? I would like to make sure we secure this into
the Nova tree as we are highly dependent on this feature.

On Tue Jun 24, 2025 at 1:28 PM JST, Christian S. Lima wrote:
<snip>
> +// SAFETY: All bit patterns are acceptable values of the types and in array case if all bit patterns
> +// are acceptable for individual values in an array, then all bit patterns are also acceptable
> +// for arrays of that type.
> +unsafe impl<T> FromBytes for T
> +where
> +    T: FromBytesSized,
> +{
> +    fn from_bytes(bytes: &[u8]) -> Option<&Self> {
> +        let slice_ptr = bytes.as_ptr().cast::<T>();
> +        if bytes.len() == ::core::mem::size_of::<T>() && slice_ptr.is_aligned() {

Another small nit: this is not a macro, so you don't need to use the
`::` prefix here. Actually even `size_of::<T>()` seems to work.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-08-01 12:26 ` Alexandre Courbot
@ 2025-08-01 15:35   ` Miguel Ojeda
  2025-08-01 17:47   ` Christian
  1 sibling, 0 replies; 13+ messages in thread
From: Miguel Ojeda @ 2025-08-01 15:35 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Christian S. Lima, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, rust-for-linux,
	linux-kernel, ~lkcamp/patches, richard120310

On Fri, Aug 1, 2025 at 3:22 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Another small nit: this is not a macro, so you don't need to use the
> `::` prefix here. Actually even `size_of::<T>()` seems to work.

Yeah, it works because Rust 1.80 moved it into the prelude. However,
we still have to support 1.78.

What we can do is adding it into our prelude meanwhile.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8] rust: transmute: Add methods for FromBytes trait
  2025-08-01 12:26 ` Alexandre Courbot
  2025-08-01 15:35   ` Miguel Ojeda
@ 2025-08-01 17:47   ` Christian
  1 sibling, 0 replies; 13+ messages in thread
From: Christian @ 2025-08-01 17:47 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
	~lkcamp/patches, richard120310

Hi, Alexandre.

> Any plan for a v9 soon? I would like to make sure we secure this into
> the Nova tree as we are highly dependent on this feature.

Yeah, I'm studying to implement the changes addressed by Kane. [1]

Thanks,
Christian

[1]  https://lore.kernel.org/rust-for-linux/CABeNrKWn4_qsAE2pqgGwVuspf7wnzT=xe_hZpuy-0HVTLD92DQ@mail.gmail.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-08-01 17:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24  4:28 [PATCH v8] rust: transmute: Add methods for FromBytes trait Christian S. Lima
2025-06-25  2:39 ` kernel test robot
2025-07-07  5:14 ` Alexandre Courbot
2025-07-14 22:16   ` Christian
2025-07-22 14:06     ` Alexandre Courbot
2025-07-25 18:37       ` Christian
2025-07-26  2:57         ` Alexandre Courbot
2025-07-27  1:37 ` Kane York
2025-07-28 19:39   ` Christian
2025-07-28 21:21     ` K. York
2025-08-01 12:26 ` Alexandre Courbot
2025-08-01 15:35   ` Miguel Ojeda
2025-08-01 17:47   ` Christian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).