Message-ID: <CAJ-ks9n-z0SETz+zBfJmda6Q_vJDeM2jmDXx48xX9qpMmR-mdQ@mail.gmail.com>
Date: Mon, 24 Mar 2025 16:55:11 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, David Gow <davidgow@...gle.com>, 
	Rae Moar <rmoar@...gle.com>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Luis Chamberlain <mcgrof@...nel.org>, Russ Weight <russ.weight@...ux.dev>, Rob Herring <robh@...nel.org>, 
	Saravana Kannan <saravanak@...gle.com>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-pci@...r.kernel.org, linux-block@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints

On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@...ton.me> wrote:
>
> On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> > Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> > lints. It also enables `clippy::as_underscore` which ensures other
> > pointer casts weren't missed. The first commit reduces the need for
> > pointer casts and is shared with another series[1].
> >
> > The final patch also enables pointer provenance lints and fixes
> > violations. See that commit message for details. The build system
> > portion of that commit is pretty messy but I couldn't find a better way
> > to convincingly ensure that these lints were applied globally.
> > Suggestions would be very welcome.
>
> I applied the patches to v6.14-rc7 and did a quick pass with
>
>     rg -nC 3 -t rust ' as ' | bat -l rust
>
> to see if there are any cases left that we could fix and I found a
> couple:
>
> * there are several cases of `number as int_type` (like `num as c_int`
>   or `my_u32 as usize` etc.) not sure what we can do about these, some
>   are probably unavoidable, but since the kernel doesn't support 16 bit
>   systems (that is true, right?), we *could* have a `From<u32> for
>   usize` impl...

Yeah, these are the most difficult ones to get rid of.

> * some instances of `'|' as u32` (samples/rust/rust_misc_device.rs:112).
>   There is a `From<char> for u32` impl, so this can just be replaced
>   with `.into()` (or maybe by using a byte literal `b'|'`?).

We can enable https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#cast_lossless
for this one.

> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>   rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>   replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>   is a clippy lint for this.

I think there's not a focused one. There's a nuclear option:
https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions

> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
>   not sure if they can be converted though (maybe they are unsizing the
>   pointer?)

I have a local series that gets rid of these by doing similar things
to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
I can send it later this week but it probably can't land until Alice
is back from vacation; she was the author of this code.

>   Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
>   one can be replaced by a `.cast()`)
>
> Some clippy lints that we could also enable that share the spirit of
> this series:
>
> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
>   above?)

It's already enabled, it's warn-by-default.

> * `cast_lossless` (maybe this catches some of the `num as int_type`
>   conversions I mentioned above)

Yeah, suggested the same above. I had hoped this would deal with the
char as u32 pattern but it did not.

> I'll leave it up to you what you want to do with this: add it to this
> series, make a new one, or let someone else handle it. If you don't want
> to handle it, let me know, then I'll create a good-first-issue :)

I'll add a patch for `cast_lossless` -- the rest should probably go
into an issue.

>
> > ---
> > Tamir Duberstein (6):
> >       rust: retain pointer mut-ness in `container_of!`
> >       rust: enable `clippy::ptr_as_ptr` lint
> >       rust: enable `clippy::ptr_cast_constness` lint
> >       rust: enable `clippy::as_ptr_cast_mut` lint
> >       rust: enable `clippy::as_underscore` lint
> >       rust: use strict provenance APIs
> >
> >  Makefile                               |   4 ++
> >  init/Kconfig                           |   3 +
> >  rust/bindings/lib.rs                   |   1 +
> >  rust/kernel/alloc.rs                   |   2 +-
> >  rust/kernel/alloc/allocator_test.rs    |   2 +-
> >  rust/kernel/alloc/kvec.rs              |   4 +-
> >  rust/kernel/block/mq/operations.rs     |   2 +-
> >  rust/kernel/block/mq/request.rs        |   7 +-
> >  rust/kernel/device.rs                  |   5 +-
> >  rust/kernel/device_id.rs               |   2 +-
> >  rust/kernel/devres.rs                  |  19 +++---
> >  rust/kernel/error.rs                   |   2 +-
> >  rust/kernel/firmware.rs                |   3 +-
> >  rust/kernel/fs/file.rs                 |   2 +-
> >  rust/kernel/io.rs                      |  16 ++---
> >  rust/kernel/kunit.rs                   |  15 ++---
> >  rust/kernel/lib.rs                     | 113 ++++++++++++++++++++++++++++++++-
> >  rust/kernel/list/impl_list_item_mod.rs |   2 +-
> >  rust/kernel/miscdevice.rs              |   2 +-
> >  rust/kernel/of.rs                      |   6 +-
> >  rust/kernel/pci.rs                     |  15 +++--
> >  rust/kernel/platform.rs                |   6 +-
> >  rust/kernel/print.rs                   |  11 ++--
> >  rust/kernel/rbtree.rs                  |  23 +++----
> >  rust/kernel/seq_file.rs                |   3 +-
> >  rust/kernel/str.rs                     |  18 ++----
> >  rust/kernel/sync/poll.rs               |   2 +-
> >  rust/kernel/uaccess.rs                 |  12 ++--
> >  rust/kernel/workqueue.rs               |  12 ++--
> >  rust/uapi/lib.rs                       |   1 +
> >  30 files changed, 218 insertions(+), 97 deletions(-)
> > ---
> > base-commit: 498f7ee4773f22924f00630136da8575f38954e8
>
> Btw I didn't find this commit anywhere I usually check, where is it
> from?

It was probably nowhere, a local frankenstein that included some fixes
from rust-fixes.

Powered by blists - more mailing lists