* [PATCH v7] rust: transmute: Add methods for FromBytes trait
@ 2025-06-15 7:20 Every2
2025-06-15 9:34 ` Miguel Ojeda
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Every2 @ 2025-06-15 7:20 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
Methods receive a slice and perform size check to add a valid way to make
conversion safe. An Option is used, in error case just return `None`.
Link: https://github.com/Rust-for-Linux/linux/issues/1119
Signed-off-by: Every2 <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
---
rust/kernel/transmute.rs | 95 +++++++++++++++++++++++++++++++++++++---
1 file changed, 89 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 1c7d43771a37..5443355de17d 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -9,29 +9,112 @@
///
/// It's okay for the type to have padding, as initializing those bytes has no effect.
///
+/// # Example
+/// ```
+/// let arr = [1, 2, 3, 4];
+///
+/// let result = u32::from_bytes(&arr);
+///
+/// #[cfg(target_endian = "little")]
+/// match result {
+/// Some(x) => assert_eq!(*x, 0x4030201),
+/// None => unreachable!()
+/// }
+///
+/// #[cfg(target_endian = "big")]
+/// match result {
+/// Some(x) => assert_eq!(*x, 0x1020304),
+/// None => unreachable!()
+/// }
+/// ```
+///
/// # 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 {
($($({$($generics:tt)*})? $t:ty, )*) => {
// SAFETY: Safety comments written in the macro invocation.
- $(unsafe impl$($($generics)*)? FromBytes for $t {})*
+ $(unsafe impl$($($generics)*)? FromBytes for $t {
+ fn from_bytes(bytes: &[u8]) -> Option<&$t> {
+ if bytes.len() == core::mem::size_of::<$t>()
+ && (bytes.as_ptr() as usize) % core::mem::align_of::<$t>() == 0
+ {
+ let slice_ptr = bytes.as_ptr().cast::<$t>();
+ unsafe { Some(&*slice_ptr) }
+ } else {
+ None
+ }
+ }
+
+ fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut $t>
+ where
+ Self: AsBytes,
+ {
+ if bytes.len() == core::mem::size_of::<$t>()
+ && (bytes.as_mut_ptr() as usize) % core::mem::align_of::<$t>() == 0
+ {
+ let slice_ptr = bytes.as_mut_ptr().cast::<$t>();
+ unsafe { Some(&mut *slice_ptr) }
+ } else {
+ None
+ }
+ }
+ })*
};
}
impl_frombytes! {
// SAFETY: All bit patterns are acceptable values of the types below.
+ // Checking the pointer size and alignment makes this operation safe and it's necessary
+ // to dereference to get the value and return it as a reference to `Self`.
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],
}
+// 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> {
+ if bytes.len() % core::mem::size_of::<T>() == 0
+ && (bytes.as_ptr() as usize) % core::mem::align_of::<T>() == 0
+ {
+ let slice_ptr = bytes.as_ptr().cast::<T>();
+ let slice_len = bytes.len() / core::mem::size_of::<T>();
+ // SAFETY: Since the code checks the size and alignment, the slice is valid.
+ unsafe { Some(core::slice::from_raw_parts(slice_ptr, slice_len)) }
+ } else {
+ None
+ }
+ }
+
+ fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
+ where
+ Self: AsBytes,
+ {
+ if bytes.len() % core::mem::size_of::<T>() == 0
+ && (bytes.as_mut_ptr() as usize) % core::mem::align_of::<T>() == 0
+ {
+ let slice_ptr = bytes.as_mut_ptr().cast::<T>();
+ let slice_len = bytes.len() / core::mem::size_of::<T>();
+ // SAFETY: Since the code checks the size and alignment, the slice is valid.
+ unsafe { Some(core::slice::from_raw_parts_mut(slice_ptr, slice_len)) }
+ } else {
+ None
+ }
+ }
+}
+
/// Types that can be viewed as an immutable slice of initialized bytes.
///
/// If a struct implements this trait, then it is okay to copy it byte-for-byte to userspace. This
--
2.49.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-15 7:20 [PATCH v7] rust: transmute: Add methods for FromBytes trait Every2
@ 2025-06-15 9:34 ` Miguel Ojeda
2025-06-16 20:07 ` Christian
2025-06-15 17:12 ` kernel test robot
2025-06-16 8:09 ` Alexandre Courbot
2 siblings, 1 reply; 14+ messages in thread
From: Miguel Ojeda @ 2025-06-15 9:34 UTC (permalink / raw)
To: Every2
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 Sun, Jun 15, 2025 at 9:20 AM Every2 <christiansantoslima21@gmail.com> wrote:
>
> Methods receive a slice and perform size check to add a valid way to make
> conversion safe. An Option is used, in error case just return `None`.
Please start with something like "Add methods ... and ... They are
useful to ...", i.e. the idea is to try to explain the "what" and the
"why" (what is being added, and why is it being added).
> Signed-off-by: Every2 <christiansantoslima21@gmail.com>
The previous version used your name in the SoB -- is this expected?
> +/// # Example
Please use the plural: # Examples
(and empty line between it and the ```).
> +/// match result {
> +/// Some(x) => assert_eq!(*x, 0x4030201),
> +/// None => unreachable!()
> +/// }
If we are going to use something like `unreachable!()`, then I would
just unwrap.
But recently we are trying to make examples look more like normal
kernel code, so please use `?` instead.
> + && (bytes.as_ptr() as usize) % core::mem::align_of::<$t>() == 0
Can we do the cast earlier and then use `.is_aligned()` instead?
Also, since this is inside a macro, we should try to avoid assuming
anything about the caller's code, so please use `::core` instead of
`core`.
> - // 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],
Don't we still want this safety comment for the array case?
Also, this is still missing safety comments on top of a couple
`unsafe` blocks (pointed out in an earlier version). Are you building
with `make ..... CLIPPY=1`?
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-15 7:20 [PATCH v7] rust: transmute: Add methods for FromBytes trait Every2
2025-06-15 9:34 ` Miguel Ojeda
@ 2025-06-15 17:12 ` kernel test robot
2025-06-16 8:09 ` Alexandre Courbot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2025-06-15 17:12 UTC (permalink / raw)
To: Every2, 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 Every2,
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-rc1 next-20250613]
[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/Every2/rust-transmute-Add-methods-for-FromBytes-trait/20250615-152228
base: https://github.com/Rust-for-Linux/linux rust-next
patch link: https://lore.kernel.org/r/20250615072042.133290-1-christiansantoslima21%40gmail.com
patch subject: [PATCH v7] rust: transmute: Add methods for FromBytes trait
config: um-randconfig-001-20250615 (https://download.01.org/0day-ci/archive/20250616/202506160013.MgafgaAm-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250616/202506160013.MgafgaAm-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/202506160013.MgafgaAm-lkp@intel.com/
All errors (new ones prefixed by >>):
>> error[E0046]: not all trait items implemented, missing: `from_bytes`, `from_mut_bytes`
--> rust/doctests_kernel_generated.rs:3156:1
|
3156 | unsafe impl kernel::transmute::FromBytes for MyStruct{};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `from_bytes`, `from_mut_bytes` in implementation
|
= help: implement the missing item: `fn from_bytes(_: &[u8]) -> Option<&Self> { todo!() }`
= help: implement the missing item: `fn from_mut_bytes<Self>(_: &mut [u8]) -> Option<&mut Self> where Self: AsBytes { todo!() }`
--
>> error[E0046]: not all trait items implemented, missing: `from_bytes`, `from_mut_bytes`
--> rust/doctests_kernel_generated.rs:3221:1
|
3221 | unsafe impl kernel::transmute::FromBytes for MyStruct{};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `from_bytes`, `from_mut_bytes` in implementation
|
= help: implement the missing item: `fn from_bytes(_: &[u8]) -> Option<&Self> { todo!() }`
= help: implement the missing item: `fn from_mut_bytes<Self>(_: &mut [u8]) -> Option<&mut Self> where Self: AsBytes { todo!() }`
--
>> error[E0599]: no function or associated item named `from_bytes` found for type `u32` in the current scope
--> rust/doctests_kernel_generated.rs:9216:19
|
9216 | let result = u32::from_bytes(&arr);
| ^^^^^^^^^^ function or associated item not found in `u32`
|
= help: items from traits can only be used if the trait is in scope
help: trait `FromBytes` which provides `from_bytes` is implemented but not in scope; perhaps you want to import it
|
3 + use crate::rust_doctest_kernel_alloc_kbox_rs_7::kernel::transmute::FromBytes;
|
help: there is an associated function `from_be` with a similar name
|
9216 | let result = u32::from_be(&arr);
| ~~~~~~~
--
>> error[E0046]: not all trait items implemented, missing: `from_bytes`, `from_mut_bytes`
--> samples/rust/rust_dma.rs:35:1
|
35 | unsafe impl kernel::transmute::FromBytes for MyStruct {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `from_bytes`, `from_mut_bytes` in implementation
|
= help: implement the missing item: `fn from_bytes(_: &[u8]) -> Option<&Self> { todo!() }`
= help: implement the missing item: `fn from_mut_bytes<Self>(_: &mut [u8]) -> Option<&mut Self> where Self: AsBytes { todo!() }`
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-15 7:20 [PATCH v7] rust: transmute: Add methods for FromBytes trait Every2
2025-06-15 9:34 ` Miguel Ojeda
2025-06-15 17:12 ` kernel test robot
@ 2025-06-16 8:09 ` Alexandre Courbot
2025-06-16 19:57 ` Christian
2 siblings, 1 reply; 14+ messages in thread
From: Alexandre Courbot @ 2025-06-16 8:09 UTC (permalink / raw)
To: Every2, 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 Sun Jun 15, 2025 at 4:20 PM JST, Every2 wrote:
> Methods receive a slice and perform size check to add a valid way to make
> conversion safe. An Option is used, in error case just return `None`.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1119
> Signed-off-by: Every2 <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
> ---
> rust/kernel/transmute.rs | 95 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 89 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> index 1c7d43771a37..5443355de17d 100644
> --- a/rust/kernel/transmute.rs
> +++ b/rust/kernel/transmute.rs
> @@ -9,29 +9,112 @@
> ///
> /// It's okay for the type to have padding, as initializing those bytes has no effect.
> ///
> +/// # Example
> +/// ```
This test won't build unless you add a
/// use kernel::transmute::FromBytes;
here.
Also, two other tests in `rust/kernel/dma.rs` break as a resulf of the new
methods added to `FromBytes`.
> +/// let arr = [1, 2, 3, 4];
> +///
> +/// let result = u32::from_bytes(&arr);
> +///
> +/// #[cfg(target_endian = "little")]
> +/// match result {
> +/// Some(x) => assert_eq!(*x, 0x4030201),
> +/// None => unreachable!()
> +/// }
> +///
> +/// #[cfg(target_endian = "big")]
> +/// match result {
> +/// Some(x) => assert_eq!(*x, 0x1020304),
> +/// None => unreachable!()
> +/// }
> +/// ```
> +///
> /// # 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 {
> ($($({$($generics:tt)*})? $t:ty, )*) => {
> // SAFETY: Safety comments written in the macro invocation.
> - $(unsafe impl$($($generics)*)? FromBytes for $t {})*
> + $(unsafe impl$($($generics)*)? FromBytes for $t {
> + fn from_bytes(bytes: &[u8]) -> Option<&$t> {
> + if bytes.len() == core::mem::size_of::<$t>()
> + && (bytes.as_ptr() as usize) % core::mem::align_of::<$t>() == 0
> + {
> + let slice_ptr = bytes.as_ptr().cast::<$t>();
> + unsafe { Some(&*slice_ptr) }
> + } else {
> + None
> + }
> + }
> +
> + fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut $t>
> + where
> + Self: AsBytes,
> + {
> + if bytes.len() == core::mem::size_of::<$t>()
> + && (bytes.as_mut_ptr() as usize) % core::mem::align_of::<$t>() == 0
> + {
> + let slice_ptr = bytes.as_mut_ptr().cast::<$t>();
> + unsafe { Some(&mut *slice_ptr) }
> + } else {
> + None
> + }
> + }
> + })*
I asked this in the previous revision [1] but didn't get a reply: why aren't we
defining this as the default implementations for `FromBytes`, since must users
will want to do exactly this anyway? I tried to do it and it failed because it
only works if `Self` is `Sized`, and we cannot conditionally implement a
default method of a trait.
We can, however, use a proxy trait that provides an implementation of
`FromBytes` for any type that is `Sized`:
pub unsafe trait FromBytesSized: Sized {}
unsafe impl<T> FromBytes for T
where
T: FromBytesSized,
{
fn from_bytes(bytes: &[u8]) -> Option<&Self> {
if bytes.len() == core::mem::size_of::<Self>()
&& (bytes.as_ptr() as usize) % core::mem::align_of::<Self>() == 0
{
let slice_ptr = bytes.as_ptr().cast::<Self>();
unsafe { Some(&*slice_ptr) }
} else {
None
}
}
fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
where
Self: AsBytes,
{
if bytes.len() == core::mem::size_of::<Self>()
&& (bytes.as_mut_ptr() as usize) % core::mem::align_of::<Self>() == 0
{
let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
unsafe { Some(&mut *slice_ptr) }
} else {
None
}
}
}
You can then implement `FromBytesSized` for all the types given to
`impl_frombytes!`.
The main benefit over the `impl_frombytes!` macro is that `FromBytesSized` is
public, and external users can just implement it on their types without having
to provide implementations for `from_bytes` and `from_mut_bytes` which would in
all likelihood be identical to the ones of `impl_frombytes!` anyway. And if
they need something different, they can always implement `FromBytes` directly.
For instance, the failing tests in `dma.rs` that I mentioned above can be fixed
by making them implement `FromBytesSized` instead of `FromBytes`.
[1] https://lore.kernel.org/rust-for-linux/D9D876NZCA5O.KFO526Q4HEED@nvidia.com/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 8:09 ` Alexandre Courbot
@ 2025-06-16 19:57 ` Christian
2025-06-17 1:15 ` Alexandre Courbot
0 siblings, 1 reply; 14+ messages in thread
From: Christian @ 2025-06-16 19:57 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.
> This test won't build unless you add a
>
> /// use kernel::transmute::FromBytes;
>
> here.
My bad, I completely forgot about it.
> I asked this in the previous revision [1] but didn't get a reply: why aren't we
> defining this as the default implementations for `FromBytes`, since must users
> will want to do exactly this anyway? I tried to do it and it failed because it
> only works if `Self` is `Sized`, and we cannot conditionally implement a
> default method of a trait.
For my part, I didn't answer your question because, IMO, I don't
really understand much of the problem and, as was pointed out before,
the implementation is incomplete, I think we're now reaching the end.
> We can, however, use a proxy trait that provides an implementation of
> `FromBytes` for any type that is `Sized`:
>
> pub unsafe trait FromBytesSized: Sized {}
>
> unsafe impl<T> FromBytes for T
> where
> T: FromBytesSized,
> {
> fn from_bytes(bytes: &[u8]) -> Option<&Self> {
> if bytes.len() == core::mem::size_of::<Self>()
> && (bytes.as_ptr() as usize) % core::mem::align_of::<Self>() == 0
> {
> let slice_ptr = bytes.as_ptr().cast::<Self>();
> unsafe { Some(&*slice_ptr) }
> } else {
> None
> }
> }
>
> fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
> where
> Self: AsBytes,
> {
> if bytes.len() == core::mem::size_of::<Self>()
> && (bytes.as_mut_ptr() as usize) % core::mem::align_of::<Self>() == 0
> {
> let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
> unsafe { Some(&mut *slice_ptr) }
> } else {
> None
> }
> }
> }
>
> You can then implement `FromBytesSized` for all the types given to
> `impl_frombytes!`.
>
> The main benefit over the `impl_frombytes!` macro is that `FromBytesSized` is
> public, and external users can just implement it on their types without having
> to provide implementations for `from_bytes` and `from_mut_bytes` which would in
> all likelihood be identical to the ones of `impl_frombytes!` anyway. And if
> they need something different, they can always implement `FromBytes` directly.
>
> For instance, the failing tests in `dma.rs` that I mentioned above can be fixed
> by making them implement `FromBytesSized` instead of `FromBytes`.
Hmm... I can change the implementation for this, but I think the idea
behind `FromBytes` and `AsBytes` is that they are the default
implementation and other parts adapt to them. Also, in the case of
Slices, since we'll use only `Sized` types, do we just abort the
conversion for them? If the maintainers are ok, I don't mind tho.
Thanks,
Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-15 9:34 ` Miguel Ojeda
@ 2025-06-16 20:07 ` Christian
2025-06-16 20:42 ` Miguel Ojeda
0 siblings, 1 reply; 14+ messages in thread
From: Christian @ 2025-06-16 20:07 UTC (permalink / raw)
To: Miguel Ojeda
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, Miguel
> Please start with something like "Add methods ... and ... They are
> useful to ...", i.e. the idea is to try to explain the "what" and the
> "why" (what is being added, and why is it being added).
I see, sorry.
> > Signed-off-by: Every2 <christiansantoslima21@gmail.com>
>
> The previous version used your name in the SoB -- is this expected?
Not at all, I checked and my git config was messy. In the next patch
everything should be fine. Thanks for the feedback
> Can we do the cast earlier and then use `.is_aligned()` instead?
Of course.
> Also, since this is inside a macro, we should try to avoid assuming
> anything about the caller's code, so please use `::core` instead of
> `core`.
I noticed that core is used in other places, is it worth using as
default dependency since other people use it and reduce verbosity of
the code?
> Also, this is still missing safety comments on top of a couple
> `unsafe` blocks (pointed out in an earlier version). Are you building
> with `make ..... CLIPPY=1`?
No, but I'll activate it now, Thanks for the valuable feedback!
Cheers,
Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 20:07 ` Christian
@ 2025-06-16 20:42 ` Miguel Ojeda
2025-06-16 21:11 ` Christian
0 siblings, 1 reply; 14+ messages in thread
From: Miguel Ojeda @ 2025-06-16 20:42 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 Mon, Jun 16, 2025 at 10:08 PM Christian
<christiansantoslima21@gmail.com> wrote:
>
> I see, sorry.
No need to apologize! It is all fine :)
> I noticed that core is used in other places, is it worth using as
> default dependency since other people use it and reduce verbosity of
> the code?
Hmm... I am not sure I understand.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 20:42 ` Miguel Ojeda
@ 2025-06-16 21:11 ` Christian
2025-06-16 21:23 ` Miguel Ojeda
0 siblings, 1 reply; 14+ messages in thread
From: Christian @ 2025-06-16 21:11 UTC (permalink / raw)
To: Miguel Ojeda
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
> Hmm... I am not sure I understand.
I mean, using something like `use core::mem;` globally since we use a
lot of core stuff and so, it would make the code less verbose just
importing size_of, align_of and etc.
Thanks,
Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 21:11 ` Christian
@ 2025-06-16 21:23 ` Miguel Ojeda
2025-06-18 19:29 ` Benno Lossin
0 siblings, 1 reply; 14+ messages in thread
From: Miguel Ojeda @ 2025-06-16 21:23 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 Mon, Jun 16, 2025 at 11:11 PM Christian
<christiansantoslima21@gmail.com> wrote:
>
> I mean, using something like `use core::mem;` globally since we use a
> lot of core stuff and so, it would make the code less verbose just
> importing size_of, align_of and etc.
Do you mean adding some more `core` bits to the `kernel` prelude?
Yeah, we are always considering which ones to add -- if you want to
propose particular ones, then it can be discussed of course. Perhaps
in Zulip?
However, since these are macros, unless we get a proper custom prelude
feature in the compiler (which we requested a long time ago), we
probably want to still to refer to everything in full, even if it is
in the `kernel` prelude.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 19:57 ` Christian
@ 2025-06-17 1:15 ` Alexandre Courbot
2025-06-17 1:55 ` Christian
0 siblings, 1 reply; 14+ messages in thread
From: Alexandre Courbot @ 2025-06-17 1:15 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 Jun 17, 2025 at 4:57 AM JST, Christian wrote:
>> We can, however, use a proxy trait that provides an implementation of
>> `FromBytes` for any type that is `Sized`:
>>
>> pub unsafe trait FromBytesSized: Sized {}
>>
>> unsafe impl<T> FromBytes for T
>> where
>> T: FromBytesSized,
>> {
>> fn from_bytes(bytes: &[u8]) -> Option<&Self> {
>> if bytes.len() == core::mem::size_of::<Self>()
>> && (bytes.as_ptr() as usize) % core::mem::align_of::<Self>() == 0
>> {
>> let slice_ptr = bytes.as_ptr().cast::<Self>();
>> unsafe { Some(&*slice_ptr) }
>> } else {
>> None
>> }
>> }
>>
>> fn from_mut_bytes(bytes: &mut [u8]) -> Option<&mut Self>
>> where
>> Self: AsBytes,
>> {
>> if bytes.len() == core::mem::size_of::<Self>()
>> && (bytes.as_mut_ptr() as usize) % core::mem::align_of::<Self>() == 0
>> {
>> let slice_ptr = bytes.as_mut_ptr().cast::<Self>();
>> unsafe { Some(&mut *slice_ptr) }
>> } else {
>> None
>> }
>> }
>> }
>>
>> You can then implement `FromBytesSized` for all the types given to
>> `impl_frombytes!`.
>>
>> The main benefit over the `impl_frombytes!` macro is that `FromBytesSized` is
>> public, and external users can just implement it on their types without having
>> to provide implementations for `from_bytes` and `from_mut_bytes` which would in
>> all likelihood be identical to the ones of `impl_frombytes!` anyway. And if
>> they need something different, they can always implement `FromBytes` directly.
>>
>> For instance, the failing tests in `dma.rs` that I mentioned above can be fixed
>> by making them implement `FromBytesSized` instead of `FromBytes`.
>
> Hmm... I can change the implementation for this, but I think the idea
> behind `FromBytes` and `AsBytes` is that they are the default
> implementation and other parts adapt to them. Also, in the case of
> Slices, since we'll use only `Sized` types, do we just abort the
> conversion for them? If the maintainers are ok, I don't mind tho.
No, for slices you just keep the implementation you currently have,
which works just fine. The idea of `FromBytesSized` is to provide a
sensible default implementation for most users who just want to morph a
stream of bytes into a given struct type.
Or if that is still unclear, consider the following doctest in `dma.rs`
that fails with this patch:
struct MyStruct { field: u32, }
// SAFETY: All bit patterns are acceptable values for `MyStruct`.
unsafe impl kernel::transmute::FromBytes for MyStruct{};
// SAFETY: Instances of `MyStruct` have no uninitialized portions.
unsafe impl kernel::transmute::AsBytes for MyStruct{};
It fails because the `FromBytes` implementation for `MyStruct` does not provide
a definition for `from_bytes` and `from_mut_bytes`. Fixing this is just a
matter of changing the `impl FromBytes` into `impl FromBytesSized`. But without
the latter, how do you make this example build without providing a definition
of `from_bytes` and `from_bytes_mut`?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-17 1:15 ` Alexandre Courbot
@ 2025-06-17 1:55 ` Christian
2025-06-17 7:39 ` Alexandre Courbot
0 siblings, 1 reply; 14+ messages in thread
From: Christian @ 2025-06-17 1:55 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
> Or if that is still unclear, consider the following doctest in `dma.rs`
> that fails with this patch:
>
> struct MyStruct { field: u32, }
>
> // SAFETY: All bit patterns are acceptable values for `MyStruct`.
> unsafe impl kernel::transmute::FromBytes for MyStruct{};
> // SAFETY: Instances of `MyStruct` have no uninitialized portions.
> unsafe impl kernel::transmute::AsBytes for MyStruct{};
>
> It fails because the `FromBytes` implementation for `MyStruct` does not provide
> a definition for `from_bytes` and `from_mut_bytes`. Fixing this is just a
> matter of changing the `impl FromBytes` into `impl FromBytesSized`. But without
> the latter, how do you make this example build without providing a definition
> of `from_bytes` and `from_bytes_mut`?
Well, that's fine with me. I'll send the next patch with this fix. Can
I put it as `suggested-by`?
Thanks,
Christian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-17 1:55 ` Christian
@ 2025-06-17 7:39 ` Alexandre Courbot
0 siblings, 0 replies; 14+ messages in thread
From: Alexandre Courbot @ 2025-06-17 7:39 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 Jun 17, 2025 at 10:55 AM JST, Christian wrote:
>> Or if that is still unclear, consider the following doctest in `dma.rs`
>> that fails with this patch:
>>
>> struct MyStruct { field: u32, }
>>
>> // SAFETY: All bit patterns are acceptable values for `MyStruct`.
>> unsafe impl kernel::transmute::FromBytes for MyStruct{};
>> // SAFETY: Instances of `MyStruct` have no uninitialized portions.
>> unsafe impl kernel::transmute::AsBytes for MyStruct{};
>>
>> It fails because the `FromBytes` implementation for `MyStruct` does not provide
>> a definition for `from_bytes` and `from_mut_bytes`. Fixing this is just a
>> matter of changing the `impl FromBytes` into `impl FromBytesSized`. But without
>> the latter, how do you make this example build without providing a definition
>> of `from_bytes` and `from_bytes_mut`?
>
> Well, that's fine with me. I'll send the next patch with this fix. Can
> I put it as `suggested-by`?
If you think it is appropriate, sure!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-16 21:23 ` Miguel Ojeda
@ 2025-06-18 19:29 ` Benno Lossin
2025-06-19 19:31 ` Miguel Ojeda
0 siblings, 1 reply; 14+ messages in thread
From: Benno Lossin @ 2025-06-18 19:29 UTC (permalink / raw)
To: Miguel Ojeda, 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 Mon Jun 16, 2025 at 11:23 PM CEST, Miguel Ojeda wrote:
> On Mon, Jun 16, 2025 at 11:11 PM Christian
> <christiansantoslima21@gmail.com> wrote:
>>
>> I mean, using something like `use core::mem;` globally since we use a
>> lot of core stuff and so, it would make the code less verbose just
>> importing size_of, align_of and etc.
>
> Do you mean adding some more `core` bits to the `kernel` prelude?
> Yeah, we are always considering which ones to add -- if you want to
> propose particular ones, then it can be discussed of course. Perhaps
> in Zulip?
>
> However, since these are macros, unless we get a proper custom prelude
> feature in the compiler (which we requested a long time ago), we
> probably want to still to refer to everything in full, even if it is
> in the `kernel` prelude.
Yes, the prelude won't have precedence over locally defined items. So if
someone does
mod core {}
your_macro_that_uses_core!(); // <-- will error with `core::mem` not existing
However in this particular case it seems like that the macro
`impl_frombytes` is a local to this file. In that case it might be fine
to rely on imports from the scope.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7] rust: transmute: Add methods for FromBytes trait
2025-06-18 19:29 ` Benno Lossin
@ 2025-06-19 19:31 ` Miguel Ojeda
0 siblings, 0 replies; 14+ messages in thread
From: Miguel Ojeda @ 2025-06-19 19:31 UTC (permalink / raw)
To: Benno Lossin
Cc: Christian, 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 Wed, Jun 18, 2025 at 9:29 PM Benno Lossin <lossin@kernel.org> wrote:
>
> Yes, the prelude won't have precedence over locally defined items. So if
> someone does
>
> mod core {}
>
> your_macro_that_uses_core!(); // <-- will error with `core::mem` not existing
Yeah, ideally the custom prelude thing (or Clippy or similar) could
warn about having any collisions with things in the custom prelude to
begin with.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-06-19 19:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-15 7:20 [PATCH v7] rust: transmute: Add methods for FromBytes trait Every2
2025-06-15 9:34 ` Miguel Ojeda
2025-06-16 20:07 ` Christian
2025-06-16 20:42 ` Miguel Ojeda
2025-06-16 21:11 ` Christian
2025-06-16 21:23 ` Miguel Ojeda
2025-06-18 19:29 ` Benno Lossin
2025-06-19 19:31 ` Miguel Ojeda
2025-06-15 17:12 ` kernel test robot
2025-06-16 8:09 ` Alexandre Courbot
2025-06-16 19:57 ` Christian
2025-06-17 1:15 ` Alexandre Courbot
2025-06-17 1:55 ` Christian
2025-06-17 7:39 ` Alexandre Courbot
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).