[PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[Miguel Ojeda]

For the new Rust support for 32-bit arm [1], Clippy warns:

    error: useless conversion to the same type: `i32`
       --> rust/kernel/error.rs:139:36
        |
    139 |         unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
        |                                    ^^^^^^^^^^^^^ help: consider removing `.into()`: `self.0`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
        = note: `-D clippy::useless-conversion` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::useless_conversion)]`

The `self.0.into()` converts an `c_int` into `ERR_PTR`'s parameter
which is a `c_long`. Thus, both types are `i32` in 32-bit. Therefore,
allow it for those architectures.

Link: https://lore.kernel.org/rust-for-linux/2dbd1491-149d-443c-9802-75786a6a3b73@gmail.com/ [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/error.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 145f5c397009..6f1587a2524e 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -135,8 +135,11 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
     /// Returns the error encoded as a pointer.
     #[allow(dead_code)]
     pub(crate) fn to_ptr<T>(self) -> *mut T {
+        #[cfg_attr(target_pointer_width = "32", allow(clippy::useless_conversion))]
         // SAFETY: `self.0` is a valid error due to its invariant.
-        unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
+        unsafe {
+            bindings::ERR_PTR(self.0.into()) as *mut _
+        }
     }
 
     /// Returns a string representing the error, if one exists.

base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.46.0

Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[Alice Ryhl]

On Tue, Jul 30, 2024 at 5:57 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> For the new Rust support for 32-bit arm [1], Clippy warns:
>
>     error: useless conversion to the same type: `i32`
>        --> rust/kernel/error.rs:139:36
>         |
>     139 |         unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
>         |                                    ^^^^^^^^^^^^^ help: consider removing `.into()`: `self.0`
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
>         = note: `-D clippy::useless-conversion` implied by `-D warnings`
>         = help: to override `-D warnings` add `#[allow(clippy::useless_conversion)]`
>
> The `self.0.into()` converts an `c_int` into `ERR_PTR`'s parameter
> which is a `c_long`. Thus, both types are `i32` in 32-bit. Therefore,
> allow it for those architectures.
>
> Link: https://lore.kernel.org/rust-for-linux/2dbd1491-149d-443c-9802-75786a6a3b73@gmail.com/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/error.rs | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 145f5c397009..6f1587a2524e 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -135,8 +135,11 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
>      /// Returns the error encoded as a pointer.
>      #[allow(dead_code)]
>      pub(crate) fn to_ptr<T>(self) -> *mut T {
> +        #[cfg_attr(target_pointer_width = "32", allow(clippy::useless_conversion))]
>          // SAFETY: `self.0` is a valid error due to its invariant.
> -        unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
> +        unsafe {
> +            bindings::ERR_PTR(self.0.into()) as *mut _
> +        }
>      }

The formatting here is a bit weird. Perhaps we should swap the cfg and
the comment?

Either way, it looks good to me.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[Miguel Ojeda]

On Tue, Jul 30, 2024 at 7:55 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> The formatting here is a bit weird. Perhaps we should swap the cfg and
> the comment?

Yeah, I don't like it either -- I reflexively did it because in the
safety series I am sending, there was a case where it would not notice
the `// SAFETY` comment if an attribute was placed afterwards.

But for blocks like this works fine, so it is not needed here, so we
can change it, even we may need to be inconsistent in a few places
about this, at least until it is fixed (assuming it is indeed a false
positive).

I created an issue in Clippy about it:
https://github.com/rust-lang/rust-clippy/issues/13189.

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[Christian Schrefl]

On 30.07.24 5:57 PM, Miguel Ojeda wrote:
> For the new Rust support for 32-bit arm [1], Clippy warns:
> 
>     error: useless conversion to the same type: `i32`
>        --> rust/kernel/error.rs:139:36
>         |
>     139 |         unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
>         |                                    ^^^^^^^^^^^^^ help: consider removing `.into()`: `self.0`
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
>         = note: `-D clippy::useless-conversion` implied by `-D warnings`
>         = help: to override `-D warnings` add `#[allow(clippy::useless_conversion)]`
> 
> The `self.0.into()` converts an `c_int` into `ERR_PTR`'s parameter
> which is a `c_long`. Thus, both types are `i32` in 32-bit. Therefore,
> allow it for those architectures.
> 
> Link: https://lore.kernel.org/rust-for-linux/2dbd1491-149d-443c-9802-75786a6a3b73@gmail.com/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/error.rs | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 145f5c397009..6f1587a2524e 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -135,8 +135,11 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
>      /// Returns the error encoded as a pointer.
>      #[allow(dead_code)]
>      pub(crate) fn to_ptr<T>(self) -> *mut T {
> +        #[cfg_attr(target_pointer_width = "32", allow(clippy::useless_conversion))]
>          // SAFETY: `self.0` is a valid error due to its invariant.
> -        unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
> +        unsafe {
> +            bindings::ERR_PTR(self.0.into()) as *mut _
> +        }
>      }
>  
>      /// Returns a string representing the error, if one exists.
> 
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
Seems good to me.

If I create another version of the arm support I'll include this there, otherwise:

Reviewd-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds

[Miguel Ojeda]

On Tue, Jul 30, 2024 at 5:57 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> For the new Rust support for 32-bit arm [1], Clippy warns:
>
>     error: useless conversion to the same type: `i32`
>        --> rust/kernel/error.rs:139:36
>         |
>     139 |         unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
>         |                                    ^^^^^^^^^^^^^ help: consider removing `.into()`: `self.0`
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
>         = note: `-D clippy::useless-conversion` implied by `-D warnings`
>         = help: to override `-D warnings` add `#[allow(clippy::useless_conversion)]`
>
> The `self.0.into()` converts an `c_int` into `ERR_PTR`'s parameter
> which is a `c_long`. Thus, both types are `i32` in 32-bit. Therefore,
> allow it for those architectures.
>
> Link: https://lore.kernel.org/rust-for-linux/2dbd1491-149d-443c-9802-75786a6a3b73@gmail.com/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Applied to `rust-next` -- thanks everyone!

    [ Fixed typo in tag. - Miguel ]

We can also change the formatting later on top of the series that will
enable that lint.

Cheers,
Miguel