Re: [PATCH v10] rust: transmute: Add methods for FromBytes trait
From: Miguel Ojeda
Date: Mon Aug 25 2025 - 10:52:31 EST
On Sun, Aug 24, 2025 at 11:31 PM Christian S. Lima
<christiansantoslima21@xxxxxxxxx> 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@xxxxxxxxxx>
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