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