* Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds
2024-07-30 15:57 [PATCH] rust: error: allow `useless_conversion` for 32-bit builds Miguel Ojeda
@ 2024-07-30 17:55 ` Alice Ryhl
2024-07-30 20:22 ` Miguel Ojeda
2024-07-30 20:36 ` Christian Schrefl
2024-08-18 21:32 ` Miguel Ojeda
2 siblings, 1 reply; 5+ messages in thread
From: Alice Ryhl @ 2024-07-30 17:55 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Russell King, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Christian Schrefl, Jamie Cunliffe, Sven Van Asbroeck,
linux-arm-kernel, rust-for-linux, linux-kernel, patches
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds
2024-07-30 17:55 ` Alice Ryhl
@ 2024-07-30 20:22 ` Miguel Ojeda
0 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2024-07-30 20:22 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Russell King,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Christian Schrefl, Jamie Cunliffe,
Sven Van Asbroeck, linux-arm-kernel, rust-for-linux, linux-kernel,
patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds
2024-07-30 15:57 [PATCH] rust: error: allow `useless_conversion` for 32-bit builds Miguel Ojeda
2024-07-30 17:55 ` Alice Ryhl
@ 2024-07-30 20:36 ` Christian Schrefl
2024-08-18 21:32 ` Miguel Ojeda
2 siblings, 0 replies; 5+ messages in thread
From: Christian Schrefl @ 2024-07-30 20:36 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Russell King
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Jamie Cunliffe, Sven Van Asbroeck,
linux-arm-kernel, rust-for-linux, linux-kernel, patches
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rust: error: allow `useless_conversion` for 32-bit builds
2024-07-30 15:57 [PATCH] rust: error: allow `useless_conversion` for 32-bit builds Miguel Ojeda
2024-07-30 17:55 ` Alice Ryhl
2024-07-30 20:36 ` Christian Schrefl
@ 2024-08-18 21:32 ` Miguel Ojeda
2 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2024-08-18 21:32 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Wedson Almeida Filho, Alex Gaynor, Russell King, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Christian Schrefl, Jamie Cunliffe, Sven Van Asbroeck,
linux-arm-kernel, rust-for-linux, linux-kernel, patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread