`FromBytes::from_bytes` comes with a few practical limitations:
- It requires the bytes slice to have the same alignment as the returned
type, which might not be guaranteed in the case of a byte stream,
- It returns a reference, requiring the returned type to implement
`Clone` if one wants to keep the value for longer than the lifetime of
the slice.
To overcome these when needed, add a `from_bytes_copy` with a default
implementation in the trait. `from_bytes_copy` returns an owned value
that is populated using an unaligned read, removing the lifetime
constraint and making it usable even on non-aligned byte slices.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/transmute.rs | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 494bb3b1d059337520efef694fc8952972d44fbf..721dd8254dcedd71ed7c1fc0ee9292950c16c89e 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -78,6 +78,23 @@ fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
None
}
}
+
+ /// Creates an owned instance of `Self` by copying `bytes`.
+ ///
+ /// As the data is copied into a properly-aligned location, this method can be used even if
+ /// [`FromBytes::from_bytes`] would return `None` due to incompatible alignment.
+ fn from_bytes_copy(bytes: &[u8]) -> Option<Self>
+ where
+ Self: Sized,
+ {
+ if bytes.len() == size_of::<Self>() {
+ // SAFETY: `bytes` has the same size as `Self`, and per the invariants of `FromBytes`,
+ // any byte sequence is a valid value for `Self`.
+ Some(unsafe { core::ptr::read_unaligned(bytes.as_ptr().cast::<Self>()) })
+ } else {
+ None
+ }
+ }
}
macro_rules! impl_frombytes {
--
2.50.1On Tue Aug 26, 2025 at 1:07 PM JST, Alexandre Courbot wrote: > `FromBytes::from_bytes` comes with a few practical limitations: > > - It requires the bytes slice to have the same alignment as the returned > type, which might not be guaranteed in the case of a byte stream, > - It returns a reference, requiring the returned type to implement > `Clone` if one wants to keep the value for longer than the lifetime of > the slice. > > To overcome these when needed, add a `from_bytes_copy` with a default > implementation in the trait. `from_bytes_copy` returns an owned value > that is populated using an unaligned read, removing the lifetime > constraint and making it usable even on non-aligned byte slices. > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> We got 3 Reviewed-by on this patch - Miguel, are you ok if I merge it together with Christian's `from_bytes` patch, since they are closely related?
On Thu, Aug 28, 2025 at 1:26 PM Alexandre Courbot <acourbot@nvidia.com> wrote: > > We got 3 Reviewed-by on this patch - Miguel, are you ok if I merge it > together with Christian's `from_bytes` patch, since they are closely > related? If you are taking this series this cycle, then sure! Acked-by: Miguel Ojeda <ojeda@kernel.org> Thanks! Cheers, Miguel
On Thu Aug 28, 2025 at 8:45 PM JST, Miguel Ojeda wrote: > On Thu, Aug 28, 2025 at 1:26 PM Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> We got 3 Reviewed-by on this patch - Miguel, are you ok if I merge it >> together with Christian's `from_bytes` patch, since they are closely >> related? > > If you are taking this series this cycle, then sure! > > Acked-by: Miguel Ojeda <ojeda@kernel.org> Thanks Miguel! Pushed this into nova-next, right after Christian's patch.
On 8/25/25 9:07 PM, Alexandre Courbot wrote:
> `FromBytes::from_bytes` comes with a few practical limitations:
>
> - It requires the bytes slice to have the same alignment as the returned
> type, which might not be guaranteed in the case of a byte stream,
> - It returns a reference, requiring the returned type to implement
> `Clone` if one wants to keep the value for longer than the lifetime of
> the slice.
>
> To overcome these when needed, add a `from_bytes_copy` with a default
> implementation in the trait. `from_bytes_copy` returns an owned value
> that is populated using an unaligned read, removing the lifetime
> constraint and making it usable even on non-aligned byte slices.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/transmute.rs | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> index 494bb3b1d059337520efef694fc8952972d44fbf..721dd8254dcedd71ed7c1fc0ee9292950c16c89e 100644
> --- a/rust/kernel/transmute.rs
> +++ b/rust/kernel/transmute.rs
> @@ -78,6 +78,23 @@ fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
> None
> }
> }
> +
> + /// Creates an owned instance of `Self` by copying `bytes`.
> + ///
> + /// As the data is copied into a properly-aligned location, this method can be used even if
> + /// [`FromBytes::from_bytes`] would return `None` due to incompatible alignment.
Some very minor suggestions:
This wording less precise than it could be: "as the data is copied" can mean
either "while it is being copied", or "because it is copied". Also, there
should not be a hyphen in "properly aligned".
I'd suggest something like this instead:
/// Unlike [`FromBytes::from_bytes`], which requires aligned input, this
/// method can be used on non-aligned data.
> + fn from_bytes_copy(bytes: &[u8]) -> Option<Self>
> + where
> + Self: Sized,
> + {
> + if bytes.len() == size_of::<Self>() {
> + // SAFETY: `bytes` has the same size as `Self`, and per the invariants of `FromBytes`,
> + // any byte sequence is a valid value for `Self`.
More wording suggestions. How about this:
// SAFETY: we just verified that `bytes` has the same size as `Self`, and per the
// invariants of `FromBytes`, any byte sequence of the correct length is a valid value
// for `Self`.
> + Some(unsafe { core::ptr::read_unaligned(bytes.as_ptr().cast::<Self>()) })
> + } else {
> + None
> + }
> + }
> }
>
> macro_rules! impl_frombytes {
>
I'm unable to find anything wrong with the code itself, and the above are just
tiny nits, so whether or not you apply either or both of the above suggestions,
please feel free to add:
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
thanks,
--
John Hubbard
On Wed Aug 27, 2025 at 9:51 AM JST, John Hubbard wrote:
> On 8/25/25 9:07 PM, Alexandre Courbot wrote:
>> `FromBytes::from_bytes` comes with a few practical limitations:
>>
>> - It requires the bytes slice to have the same alignment as the returned
>> type, which might not be guaranteed in the case of a byte stream,
>> - It returns a reference, requiring the returned type to implement
>> `Clone` if one wants to keep the value for longer than the lifetime of
>> the slice.
>>
>> To overcome these when needed, add a `from_bytes_copy` with a default
>> implementation in the trait. `from_bytes_copy` returns an owned value
>> that is populated using an unaligned read, removing the lifetime
>> constraint and making it usable even on non-aligned byte slices.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> rust/kernel/transmute.rs | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
>> index 494bb3b1d059337520efef694fc8952972d44fbf..721dd8254dcedd71ed7c1fc0ee9292950c16c89e 100644
>> --- a/rust/kernel/transmute.rs
>> +++ b/rust/kernel/transmute.rs
>> @@ -78,6 +78,23 @@ fn from_bytes_mut(bytes: &mut [u8]) -> Option<&mut Self>
>> None
>> }
>> }
>> +
>> + /// Creates an owned instance of `Self` by copying `bytes`.
>> + ///
>> + /// As the data is copied into a properly-aligned location, this method can be used even if
>> + /// [`FromBytes::from_bytes`] would return `None` due to incompatible alignment.
>
> Some very minor suggestions:
>
> This wording less precise than it could be: "as the data is copied" can mean
> either "while it is being copied", or "because it is copied". Also, there
> should not be a hyphen in "properly aligned".
>
> I'd suggest something like this instead:
>
> /// Unlike [`FromBytes::from_bytes`], which requires aligned input, this
> /// method can be used on non-aligned data.
That's much simpler and better. I'll just add "... at the cost of a
copy." to not lose this information.
>
>> + fn from_bytes_copy(bytes: &[u8]) -> Option<Self>
>> + where
>> + Self: Sized,
>> + {
>> + if bytes.len() == size_of::<Self>() {
>> + // SAFETY: `bytes` has the same size as `Self`, and per the invariants of `FromBytes`,
>> + // any byte sequence is a valid value for `Self`.
>
> More wording suggestions. How about this:
>
> // SAFETY: we just verified that `bytes` has the same size as `Self`, and per the
> // invariants of `FromBytes`, any byte sequence of the correct length is a valid value
> // for `Self`.
Taken as-is, thanks!
On Tue Aug 26, 2025 at 6:07 AM CEST, Alexandre Courbot wrote: > `FromBytes::from_bytes` comes with a few practical limitations: > > - It requires the bytes slice to have the same alignment as the returned > type, which might not be guaranteed in the case of a byte stream, > - It returns a reference, requiring the returned type to implement > `Clone` if one wants to keep the value for longer than the lifetime of > the slice. I think that any `Sized` type that can implement `FromBytes` also can implement `Copy`, so I don't feel like the last point is a strong one. But the unaligned byte stream is motivation enough :) > To overcome these when needed, add a `from_bytes_copy` with a default > implementation in the trait. `from_bytes_copy` returns an owned value > that is populated using an unaligned read, removing the lifetime > constraint and making it usable even on non-aligned byte slices. > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> Reviewed-by: Benno Lossin <lossin@kernel.org> --- Cheers, Benno > --- > rust/kernel/transmute.rs | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+)
© 2016 - 2025 Red Hat, Inc.