linux-kernel - Re: [PATCH 02/13] rust: init: simplify from `map_err` to `inspect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ap7u5hpgfdzYgTkobIleYc0uolBIsAVbq56CwMJG6k4x57wXeEtu4fpLpgSOvQtbJHvB6D0J1wlBaqDV-xt2owlVejhfABUv_ivILyJ2J9o=@protonmail.com>
Date: Mon, 01 Jul 2024 20:05:26 +0000
From: Björn Roy Baron <bjorn3_gh@...tonmail.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: Wedson Almeida Filho <wedsonaf@...il.com>, Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl <aliceryhl@...gle.com>, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 02/13] rust: init: simplify from `map_err` to `inspect_err`

On Monday, July 1st, 2024 at 20:36, Miguel Ojeda <ojeda@...nel.org> wrote:

> A new complexity lint, `manual_inspect` [1], has been introduced in
> the upcoming Rust 1.81 (currently in nightly), which checks for uses of
> `map*` which return the original item:
> 
>     error:
>     --> rust/kernel/init.rs:846:23
>         |
>     846 |         (self.1)(val).map_err(|e| {
>         |                       ^^^^^^^
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
>         = note: `-D clippy::manual-inspect` implied by `-D warnings`
>         = help: to override `-D warnings` add `#[allow(clippy::manual_inspect)]`
>     help: try
>         |
>     846 ~         (self.1)(val).inspect_err(|e| {
>     847 |             // SAFETY: `slot` was initialized above.
>     848 ~             unsafe { core::ptr::drop_in_place(slot) };
>         |
> 
> Thus clean them up.
> 
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/manual_inspect [1]
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---
>  rust/kernel/init.rs | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 68605b633e73..07bacf134c19 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -843,11 +843,9 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
>          let val = unsafe { &mut *slot };
>          // SAFETY: `slot` is considered pinned.
>          let val = unsafe { Pin::new_unchecked(val) };
> -        (self.1)(val).map_err(|e| {
> +        (self.1)(val).inspect_err(|_|
>              // SAFETY: `slot` was initialized above.
> -            unsafe { core::ptr::drop_in_place(slot) };
> -            e
> -        })
> +            unsafe { core::ptr::drop_in_place(slot) })

The formatting here is a bit weird. I would have expected the ) ending the inspect_err call to be placed on a new line given that the unsafe block isn't placed on the same line as the start of the inspect_err call either. This seems to be a case where rustfmt mostly gives up on formatting. If I deindent the comment and unsafe block rustfmt will keep it that way and if I add a trailing space to the inspect_err line it will keep it and emit an error that it left behind trailing whitespace. Maybe add a block around the comment and unsafe block to make rustfmt work again?

>      }
>  }
> 
> @@ -941,11 +939,9 @@ unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
>          // SAFETY: All requirements fulfilled since this function is `__init`.
>          unsafe { self.0.__pinned_init(slot)? };
>          // SAFETY: The above call initialized `slot` and we still have unique access.
> -        (self.1)(unsafe { &mut *slot }).map_err(|e| {
> +        (self.1)(unsafe { &mut *slot }).inspect_err(|_|
>              // SAFETY: `slot` was initialized above.
> -            unsafe { core::ptr::drop_in_place(slot) };
> -            e
> -        })
> +            unsafe { core::ptr::drop_in_place(slot) })
>      }
>  }
> 
> --
> 2.45.2