* [PATCH v2 0/5] rust: reduce pointer casts, enable related lints
@ 2025-03-09 16:00 Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
` (5 more replies)
0 siblings, 6 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
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].
Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v2:
- Fixed typo in first commit message.
- Added additional patches, converted to series.
- Link to v1: https://lore.kernel.org/r/20250307-ptr-as-ptr-v1-1-582d06514c98@gmail.com
---
Tamir Duberstein (5):
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
Makefile | 4 ++++
rust/bindings/lib.rs | 1 +
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 | 2 +-
rust/kernel/fs/file.rs | 2 +-
rust/kernel/io.rs | 18 +++++++++---------
rust/kernel/kunit.rs | 15 +++++++--------
rust/kernel/lib.rs | 5 ++---
rust/kernel/list/impl_list_item_mod.rs | 2 +-
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/of.rs | 6 +++---
rust/kernel/pci.rs | 13 ++++++++-----
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 | 10 +++++-----
rust/kernel/sync/poll.rs | 2 +-
rust/kernel/workqueue.rs | 12 ++++++------
rust/uapi/lib.rs | 1 +
27 files changed, 95 insertions(+), 86 deletions(-)
---
base-commit: ff64846bee0e7e3e7bc9363ebad3bab42dd27e24
change-id: 20250307-ptr-as-ptr-21b1867fc4d4
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!`
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
@ 2025-03-09 16:00 ` Tamir Duberstein
2025-03-12 14:22 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
` (4 subsequent siblings)
5 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
Avoid casting the input pointer to `*const _`, allowing the output
pointer to be `*mut` if the input is `*mut`. This allows a number of
`*const` to `*mut` conversions to be removed at the cost of slightly
worse ergonomics when the macro is used with a reference rather than a
pointer; the only example of this was in the macro's own doctest.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/lib.rs | 5 ++---
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 2 +-
rust/kernel/rbtree.rs | 23 ++++++++++-------------
4 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7697c60b2d1a..9cd6b6864739 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -187,7 +187,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
/// }
///
/// let test = Test { a: 10, b: 20 };
-/// let b_ptr = &test.b;
+/// let b_ptr: *const _ = &test.b;
/// // SAFETY: The pointer points at the `b` field of a `Test`, so the resulting pointer will be
/// // in-bounds of the same allocation as `b_ptr`.
/// let test_alias = unsafe { container_of!(b_ptr, Test, b) };
@@ -196,9 +196,8 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
#[macro_export]
macro_rules! container_of {
($ptr:expr, $type:ty, $($f:tt)*) => {{
- let ptr = $ptr as *const _ as *const u8;
let offset: usize = ::core::mem::offset_of!($type, $($f)*);
- ptr.sub(offset) as *const $type
+ $ptr.byte_sub(offset).cast::<$type>()
}}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4c98b5b9aa1e..c37476576f02 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -364,7 +364,7 @@ pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::pci_dev {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct pci_dev`.
- unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+ unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) }
}
/// Returns the PCI vendor ID.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 50e6b0421813..c51617569c01 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -189,7 +189,7 @@ unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::platform_device {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct platform_device`.
- unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut()
+ unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }
}
}
diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 0d1e75810664..1fdea2806cfa 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -424,7 +424,7 @@ pub fn cursor_lower_bound(&mut self, key: &K) -> Option<Cursor<'_, K, V>>
while !node.is_null() {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node, Node<K, V>, links) };
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
let this_key = unsafe { &(*this).key };
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
@@ -496,7 +496,7 @@ fn drop(&mut self) {
// but it is not observable. The loop invariant is still maintained.
// SAFETY: `this` is valid per the loop invariant.
- unsafe { drop(KBox::from_raw(this.cast_mut())) };
+ unsafe { drop(KBox::from_raw(this)) };
}
}
}
@@ -761,7 +761,7 @@ pub fn remove_current(self) -> (Option<Self>, RBTreeNode<K, V>) {
let next = self.get_neighbor_raw(Direction::Next);
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
let node = RBTreeNode { node };
@@ -806,7 +806,7 @@ fn remove_neighbor(&mut self, direction: Direction) -> Option<RBTreeNode<K, V>>
unsafe { bindings::rb_erase(neighbor, addr_of_mut!(self.tree.root)) };
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(neighbor, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(neighbor, Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
return Some(RBTreeNode { node });
@@ -912,7 +912,7 @@ unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b
unsafe fn to_key_value_raw<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, *mut V) {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) };
// SAFETY: The passed `node` is the current node or a non-null neighbor,
// thus `this` is valid by the type invariants.
let k = unsafe { &(*this).key };
@@ -1021,7 +1021,7 @@ fn next(&mut self) -> Option<Self::Item> {
// SAFETY: By the type invariant of `IterRaw`, `self.next` is a valid node in an `RBTree`,
// and by the type invariant of `RBTree`, all nodes point to the links field of `Node<K, V>` objects.
- let cur = unsafe { container_of!(self.next, Node<K, V>, links) }.cast_mut();
+ let cur = unsafe { container_of!(self.next, Node<K, V>, links) };
// SAFETY: `self.next` is a valid tree node by the type invariants.
self.next = unsafe { bindings::rb_next(self.next) };
@@ -1216,7 +1216,7 @@ pub fn get_mut(&mut self) -> &mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - We have exclusive access to the underlying tree, and can thus give out a mutable reference.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Converts the entry into a mutable reference to its value.
@@ -1226,7 +1226,7 @@ pub fn into_mut(self) -> &'a mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - This consumes the `&'a mut RBTree<K, V>`, therefore it can give out a mutable reference that lives for `'a`.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Remove this entry from the [`RBTree`].
@@ -1239,9 +1239,7 @@ pub fn remove_node(self) -> RBTreeNode<K, V> {
RBTreeNode {
// SAFETY: The node was a node in the tree, but we removed it, so we can convert it
// back into a box.
- node: unsafe {
- KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut())
- },
+ node: unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) },
}
}
@@ -1272,8 +1270,7 @@ fn replace(self, node: RBTreeNode<K, V>) -> RBTreeNode<K, V> {
// SAFETY:
// - `self.node_ptr` produces a valid pointer to a node in the tree.
// - Now that we removed this entry from the tree, we can convert the node to a box.
- let old_node =
- unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut()) };
+ let old_node = unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) };
RBTreeNode { node: old_node }
}
--
2.48.1
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
@ 2025-03-09 16:00 ` Tamir Duberstein
2025-03-12 14:39 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
` (3 subsequent siblings)
5 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast` is safer because it cannot accidentally change the
> pointer's mutability, nor cast the pointer to other types like `usize`.
There are a few classes of changes required:
- Modules generated by bindgen are marked
`#[allow(clippy::ptr_as_ptr)]`.
- Inferred casts (` as _`) are replaced with `.cast()`.
- Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
- Multistep casts from references (` as *const _ as *const T`) are
replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
according to the previous rules. The intermediate `let` binding is
required because `(x as *const _).cast::<T>()` results in inference
failure.
- Native literal C strings are replaced with `c_str!().as_char_ptr()`.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/bindings/lib.rs | 1 +
rust/kernel/alloc/allocator_test.rs | 2 +-
rust/kernel/alloc/kvec.rs | 4 ++--
rust/kernel/device.rs | 5 +++--
rust/kernel/devres.rs | 2 +-
rust/kernel/error.rs | 2 +-
rust/kernel/fs/file.rs | 2 +-
rust/kernel/kunit.rs | 15 +++++++--------
rust/kernel/list/impl_list_item_mod.rs | 2 +-
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 4 +++-
rust/kernel/print.rs | 11 +++++------
rust/kernel/seq_file.rs | 3 ++-
rust/kernel/str.rs | 2 +-
rust/kernel/sync/poll.rs | 2 +-
rust/kernel/workqueue.rs | 10 +++++-----
rust/uapi/lib.rs | 1 +
18 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/Makefile b/Makefile
index 70bdbf2218fc..ec8efc8e23ba 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,7 @@ export rust_common_flags := --edition=2021 \
-Wclippy::needless_continue \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
+ -Wclippy::ptr_as_ptr \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 014af0d1fc70..0486a32ed314 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -25,6 +25,7 @@
)]
#[allow(dead_code)]
+#[allow(clippy::ptr_as_ptr)]
#[allow(clippy::undocumented_unsafe_blocks)]
mod bindings_raw {
// Manual definition for blocklisted types.
diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs
index c37d4c0c64e9..8017aa9d5213 100644
--- a/rust/kernel/alloc/allocator_test.rs
+++ b/rust/kernel/alloc/allocator_test.rs
@@ -82,7 +82,7 @@ unsafe fn realloc(
// SAFETY: Returns either NULL or a pointer to a memory allocation that satisfies or
// exceeds the given size and alignment requirements.
- let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) } as *mut u8;
+ let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) }.cast::<u8>();
let dst = NonNull::new(dst).ok_or(AllocError)?;
if flags.contains(__GFP_ZERO) {
diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index ae9d072741ce..c12844764671 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -262,7 +262,7 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
// - `self.len` is smaller than `self.capacity` and hence, the resulting pointer is
// guaranteed to be part of the same allocated object.
// - `self.len` can not overflow `isize`.
- let ptr = unsafe { self.as_mut_ptr().add(self.len) } as *mut MaybeUninit<T>;
+ let ptr = unsafe { self.as_mut_ptr().add(self.len) }.cast::<MaybeUninit<T>>();
// SAFETY: The memory between `self.len` and `self.capacity` is guaranteed to be allocated
// and valid, but uninitialized.
@@ -554,7 +554,7 @@ fn drop(&mut self) {
// - `ptr` points to memory with at least a size of `size_of::<T>() * len`,
// - all elements within `b` are initialized values of `T`,
// - `len` does not exceed `isize::MAX`.
- unsafe { Vec::from_raw_parts(ptr as _, len, len) }
+ unsafe { Vec::from_raw_parts(ptr.cast(), len, len) }
}
}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..9e500498835d 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -168,16 +168,17 @@ pub fn pr_dbg(&self, args: fmt::Arguments<'_>) {
/// `KERN_*`constants, for example, `KERN_CRIT`, `KERN_ALERT`, etc.
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
+ let msg: *const _ = &msg;
// SAFETY: `klevel` is null-terminated and one of the kernel constants. `self.as_raw`
// is valid because `self` is valid. The "%pA" format string expects a pointer to
// `fmt::Arguments`, which is what we're passing as the last argument.
#[cfg(CONFIG_PRINTK)]
unsafe {
bindings::_dev_printk(
- klevel as *const _ as *const crate::ffi::c_char,
+ klevel.as_ptr().cast::<crate::ffi::c_char>(),
self.as_raw(),
c_str!("%pA").as_char_ptr(),
- &msg as *const _ as *const crate::ffi::c_void,
+ msg.cast::<crate::ffi::c_void>(),
)
};
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 942376f6f3af..3a9d998ec371 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -157,7 +157,7 @@ fn remove_action(this: &Arc<Self>) {
#[allow(clippy::missing_safety_doc)]
unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
- let ptr = ptr as *mut DevresInner<T>;
+ let ptr = ptr.cast::<DevresInner<T>>();
// Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
// reference.
// SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index f6ecf09cb65f..8654d52b0bb9 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _) as *mut _ }
+ unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index e03dbe14d62a..8936afc234a4 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -364,7 +364,7 @@ fn deref(&self) -> &LocalFile {
//
// By the type invariants, there are no `fdget_pos` calls that did not take the
// `f_pos_lock` mutex.
- unsafe { LocalFile::from_raw_file(self as *const File as *const bindings::file) }
+ unsafe { LocalFile::from_raw_file((self as *const Self).cast()) }
}
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 824da0e9738a..7ed2063c1af0 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -8,19 +8,20 @@
use core::{ffi::c_void, fmt};
+#[cfg(CONFIG_PRINTK)]
+use crate::c_str;
+
/// Prints a KUnit error-level message.
///
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn err(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x013%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x013%pA").as_char_ptr(), args.cast::<c_void>());
}
}
@@ -29,14 +30,12 @@ pub fn err(args: fmt::Arguments<'_>) {
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn info(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x016%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x016%pA").as_char_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee1..1f9498c1458f 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -34,7 +34,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
// SAFETY: The caller promises that the pointer is valid. The implementer promises that the
// `OFFSET` constant is correct.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast() }
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c37476576f02..63218abb7a25 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -75,7 +75,7 @@ extern "C" fn probe_callback(
// Let the `struct pci_dev` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct pci_dev`.
- unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign().cast()) };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index c51617569c01..d609119e8ce8 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -66,7 +66,9 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
// Let the `struct platform_device` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct platform_device`.
- unsafe { bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe {
+ bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign().cast())
+ };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index b19ee490be58..0245c145ea32 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -25,7 +25,7 @@
// SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
// SAFETY: TODO.
- let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
+ let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
w.pos().cast()
}
@@ -102,6 +102,7 @@ pub unsafe fn call_printk(
module_name: &[u8],
args: fmt::Arguments<'_>,
) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
#[cfg(CONFIG_PRINTK)]
// SAFETY: TODO.
@@ -109,7 +110,7 @@ pub unsafe fn call_printk(
bindings::_printk(
format_string.as_ptr(),
module_name.as_ptr(),
- &args as *const _ as *const c_void,
+ args.cast::<c_void>(),
);
}
}
@@ -122,15 +123,13 @@ pub unsafe fn call_printk(
#[doc(hidden)]
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
pub fn call_printk_cont(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
//
// SAFETY: The format string is fixed.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- format_strings::CONT.as_ptr(),
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(format_strings::CONT.as_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 04947c672979..90545d28e6b7 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -31,12 +31,13 @@ pub unsafe fn from_raw<'a>(ptr: *mut bindings::seq_file) -> &'a SeqFile {
/// Used by the [`seq_print`] macro.
pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: Passing a void pointer to `Arguments` is valid for `%pA`.
unsafe {
bindings::seq_printf(
self.inner.get(),
c_str!("%pA").as_char_ptr(),
- &args as *const _ as *const crate::ffi::c_void,
+ args.cast::<crate::ffi::c_void>(),
);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..6a1a982b946d 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -191,7 +191,7 @@ pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
// to a `NUL`-terminated C string.
let len = unsafe { bindings::strlen(ptr) } + 1;
// SAFETY: Lifetime guaranteed by the safety precondition.
- let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len) };
+ let bytes = unsafe { core::slice::from_raw_parts(ptr.cast(), len) };
// SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`.
// As we have added 1 to `len`, the last byte is known to be `NUL`.
unsafe { Self::from_bytes_with_nul_unchecked(bytes) }
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index d5f17153b424..a151f54cde91 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -73,7 +73,7 @@ pub fn register_wait(&mut self, file: &File, cv: &PollCondVar) {
// be destroyed, the destructor must run. That destructor first removes all waiters,
// and then waits for an rcu grace period. Therefore, `cv.wait_queue_head` is valid for
// long enough.
- unsafe { qproc(file.as_ptr() as _, cv.wait_queue_head.get(), self.0.get()) };
+ unsafe { qproc(file.as_ptr().cast(), cv.wait_queue_head.get(), self.0.get()) };
}
}
}
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 0cd100d2aefb..8ff54105be3f 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -170,7 +170,7 @@ impl Queue {
pub unsafe fn from_raw<'a>(ptr: *const bindings::workqueue_struct) -> &'a Queue {
// SAFETY: The `Queue` type is `#[repr(transparent)]`, so the pointer cast is valid. The
// caller promises that the pointer is not dangling.
- unsafe { &*(ptr as *const Queue) }
+ unsafe { &*ptr.cast::<Queue>() }
}
/// Enqueues a work item.
@@ -457,7 +457,7 @@ fn get_work_offset(&self) -> usize {
#[inline]
unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work<T, ID> {
// SAFETY: The caller promises that the pointer is valid.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut Work<T, ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast::<Work<T, ID>>() }
}
/// Returns a pointer to the struct containing the [`Work<T, ID>`] field.
@@ -472,7 +472,7 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
{
// SAFETY: The caller promises that the pointer points at a field of the right type in the
// right kind of struct.
- unsafe { (ptr as *mut u8).sub(Self::OFFSET) as *mut Self }
+ unsafe { ptr.cast::<u8>().sub(Self::OFFSET).cast::<Self>() }
}
}
@@ -538,7 +538,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
@@ -591,7 +591,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 13495910271f..fe9bf7b5a306 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -15,6 +15,7 @@
#![allow(
clippy::all,
clippy::undocumented_unsafe_blocks,
+ clippy::ptr_as_ptr,
dead_code,
missing_docs,
non_camel_case_types,
--
2.48.1
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
@ 2025-03-09 16:00 ` Tamir Duberstein
2025-03-12 14:40 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
` (2 subsequent siblings)
5 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.72.0, Clippy introduced the `ptr_cast_constness` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast_mut` and `pointer::cast_const` are safer because they
> cannot accidentally cast the pointer to another type.
There are only 2 affected sites:
- `*mut T as *const U as *mut U` becomes `(*mut T).cast()`
- `&self as *const Self as *mut Self` becomes a reference-to-pointer
coercion + `(*const Self).cast()`.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/request.rs | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index ec8efc8e23ba..c62bae2b107b 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,7 @@ export rust_common_flags := --edition=2021 \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
-Wclippy::ptr_as_ptr \
+ -Wclippy::ptr_cast_constness \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 7943f43b9575..10c6d69be7f3 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -69,7 +69,7 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef<Self> {
// INVARIANT: By the safety requirements of this function, invariants are upheld.
// SAFETY: By the safety requirement of this function, we own a
// reference count that we can pass to `ARef`.
- unsafe { ARef::from_raw(NonNull::new_unchecked(ptr as *const Self as *mut Self)) }
+ unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) }
}
/// Notify the block layer that a request is going to be processed now.
@@ -151,11 +151,12 @@ pub(crate) unsafe fn wrapper_ptr(this: *mut Self) -> NonNull<RequestDataWrapper>
/// Return a reference to the [`RequestDataWrapper`] stored in the private
/// area of the request structure.
pub(crate) fn wrapper_ref(&self) -> &RequestDataWrapper {
+ let this: *const _ = self;
// SAFETY: By type invariant, `self.0` is a valid allocation. Further,
// the private data associated with this request is initialized and
// valid. The existence of `&self` guarantees that the private data is
// valid as a shared reference.
- unsafe { Self::wrapper_ptr(self as *const Self as *mut Self).as_ref() }
+ unsafe { Self::wrapper_ptr(this.cast_mut()).as_ref() }
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (2 preceding siblings ...)
2025-03-09 16:00 ` [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
@ 2025-03-09 16:00 ` Tamir Duberstein
2025-03-11 13:08 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint Tamir Duberstein
2025-03-12 15:07 ` [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Benno Lossin
5 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.66.0, Clippy introduced the `as_ptr_cast_mut` lint [1]:
> Since `as_ptr` takes a `&self`, the pointer won’t have write
> permissions unless interior mutability is used, making it unlikely
> that having it as a mutable pointer is correct.
There is only one affected callsite, and the change amounts to replacing
`as _` with `.cast_mut().cast()`. This doesn't change the semantics, but
is more descriptive of what's going on.
Apply this change and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/devres.rs | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c62bae2b107b..bb15b86182a3 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ export rust_common_flags := --edition=2021 \
-Wrust_2018_idioms \
-Wunreachable_pub \
-Wclippy::all \
+ -Wclippy::as_ptr_cast_mut \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 3a9d998ec371..598001157293 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -143,7 +143,7 @@ fn remove_action(this: &Arc<Self>) {
bindings::devm_remove_action_nowarn(
this.dev.as_raw(),
Some(this.callback),
- this.as_ptr() as _,
+ this.as_ptr().cast_mut().cast(),
)
};
--
2.48.1
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (3 preceding siblings ...)
2025-03-09 16:00 ` [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
@ 2025-03-09 16:00 ` Tamir Duberstein
2025-03-12 15:05 ` Benno Lossin
2025-03-12 15:07 ` [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Benno Lossin
5 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-09 16:00 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.63.0, Clippy introduced the `as_underscore` lint [1]:
> The conversion might include lossy conversion or a dangerous cast that
> might go undetected due to the type being inferred.
>
> The lint is allowed by default as using `_` is less wordy than always
> specifying the type.
Always specifying the type is especially helpful in function call
contexts where the inferred type may change at a distance. Specifying
the type also allows Clippy to spot more cases of `useless_conversion`.
The primary downside is the need to specify the type in trivial getters.
There are 4 such functions: 3 have become slightly less ergonomic, 1 was
revealed to be a `useless_conversion`.
While this doesn't eliminate unchecked `as` conversions, it makes such
conversions easier to scrutinize. It also has the slight benefit of
removing a degree of freedom on which to bikeshed. Thus apply the
changes and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/operations.rs | 2 +-
rust/kernel/block/mq/request.rs | 2 +-
rust/kernel/device_id.rs | 2 +-
rust/kernel/devres.rs | 15 ++++++++-------
rust/kernel/error.rs | 2 +-
rust/kernel/firmware.rs | 2 +-
rust/kernel/io.rs | 18 +++++++++---------
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/of.rs | 6 +++---
rust/kernel/pci.rs | 9 ++++++---
rust/kernel/str.rs | 8 ++++----
rust/kernel/workqueue.rs | 2 +-
13 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/Makefile b/Makefile
index bb15b86182a3..2af40bfed9ce 100644
--- a/Makefile
+++ b/Makefile
@@ -478,6 +478,7 @@ export rust_common_flags := --edition=2021 \
-Wunreachable_pub \
-Wclippy::all \
-Wclippy::as_ptr_cast_mut \
+ -Wclippy::as_underscore \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 864ff379dc91..d18ef55490da 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -101,7 +101,7 @@ impl<T: Operations> OperationsVTable<T> {
if let Err(e) = ret {
e.to_blk_status()
} else {
- bindings::BLK_STS_OK as _
+ bindings::BLK_STS_OK as u8
}
}
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 10c6d69be7f3..bcf2b73d9189 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -125,7 +125,7 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
// success of the call to `try_set_end` guarantees that there are no
// `ARef`s pointing to this request. Therefore it is safe to hand it
// back to the block layer.
- unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as _) };
+ unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as u8) };
Ok(())
}
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index e5859217a579..4063f09d76d9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -82,7 +82,7 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
unsafe {
raw_ids[i]
.as_mut_ptr()
- .byte_offset(T::DRIVER_DATA_OFFSET as _)
+ .byte_add(T::DRIVER_DATA_OFFSET)
.cast::<usize>()
.write(i);
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 598001157293..20159b7c9293 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -45,7 +45,7 @@ struct DevresInner<T> {
/// # Example
///
/// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -59,19 +59,19 @@ struct DevresInner<T> {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -115,8 +115,9 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
// SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
// detached.
- let ret =
- unsafe { bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data as _) };
+ let ret = unsafe {
+ bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data.cast_mut().cast())
+ };
if ret != 0 {
// SAFETY: We just created another reference to `inner` in order to pass it to
@@ -130,7 +131,7 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
}
fn as_ptr(&self) -> *const Self {
- self as _
+ self
}
fn remove_action(this: &Arc<Self>) {
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 8654d52b0bb9..eb8fa52f08ba 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
+ unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..9a279330261c 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -61,7 +61,7 @@ fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
// SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
// `name` and `dev` are valid as by their type invariants.
- let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
+ let ret = unsafe { func.0(pfw, name.as_char_ptr(), dev.as_raw()) };
if ret != 0 {
return Err(Error::from_errno(ret));
}
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d4a73e52e3ee..d375eed73dc8 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -5,7 +5,7 @@
//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
use crate::error::{code::EINVAL, Result};
-use crate::{bindings, build_assert};
+use crate::{bindings, build_assert, ffi::c_void};
/// Raw representation of an MMIO region.
///
@@ -56,7 +56,7 @@ pub fn maxsize(&self) -> usize {
/// # Examples
///
/// ```no_run
-/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -70,19 +70,19 @@ pub fn maxsize(&self) -> usize {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(addr as _) }
+ unsafe { bindings::$name(addr as *const c_void) }
}
/// Read IO data from a given offset.
@@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$name(addr as _) })
+ Ok(unsafe { bindings::$name(addr as *const c_void) })
}
};
}
@@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _, ) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
}
/// Write IO data from a given offset.
@@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
Ok(())
}
};
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d..2c66e926bffb 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -33,7 +33,7 @@ impl MiscDeviceOptions {
pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
// SAFETY: All zeros is valid for this C type.
let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
- result.minor = bindings::MISC_DYNAMIC_MINOR as _;
+ result.minor = bindings::MISC_DYNAMIC_MINOR as i32;
result.name = self.name.as_char_ptr();
result.fops = create_vtable::<T>();
result
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 04f2d8ef29cb..40d1bd13682c 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- self.0.data as _
+ self.0.data as usize
}
}
@@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
// SAFETY: FFI type is valid to be zero-initialized.
let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
- // TODO: Use `clone_from_slice` once the corresponding types do match.
+ // TODO: Use `copy_from_slice` once stabilized for `const`.
let mut i = 0;
while i < src.len() {
- of.compatible[i] = src[i] as _;
+ of.compatible[i] = src[i];
i += 1;
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 63218abb7a25..a925732f6c7a 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -166,7 +166,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
fn index(&self) -> usize {
- self.0.driver_data as _
+ self.0.driver_data
}
}
@@ -201,7 +201,10 @@ macro_rules! pci_device_table {
/// MODULE_PCI_TABLE,
/// <MyDriver as pci::Driver>::IdInfo,
/// [
-/// (pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as _), ())
+/// (
+/// pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as u32),
+/// (),
+/// )
/// ]
/// );
///
@@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
// `ioptr` is valid by the safety requirements.
// `num` is valid by the safety requirements.
unsafe {
- bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
+ bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
bindings::pci_release_region(pdev.as_raw(), num);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6a1a982b946d..0b80a119d5f0 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -692,9 +692,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: pos as _,
- pos: pos as _,
- end: end as _,
+ beg: pos as usize,
+ pos: pos as usize,
+ end: end as usize,
}
}
@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
///
/// N.B. It may point to invalid memory.
pub(crate) fn pos(&self) -> *mut u8 {
- self.pos as _
+ self.pos as *mut u8
}
/// Returns the number of bytes written to the formatter.
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 8ff54105be3f..d03f3440cb5a 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -198,7 +198,7 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
unsafe {
w.__enqueue(move |work_ptr| {
bindings::queue_work_on(
- bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
+ bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32,
queue_ptr,
work_ptr,
)
--
2.48.1
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint
2025-03-09 16:00 ` [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
@ 2025-03-11 13:08 ` Benno Lossin
0 siblings, 0 replies; 38+ messages in thread
From: Benno Lossin @ 2025-03-11 13:08 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> In Rust 1.66.0, Clippy introduced the `as_ptr_cast_mut` lint [1]:
>
>> Since `as_ptr` takes a `&self`, the pointer won’t have write
>> permissions unless interior mutability is used, making it unlikely
>> that having it as a mutable pointer is correct.
>
> There is only one affected callsite, and the change amounts to replacing
> `as _` with `.cast_mut().cast()`. This doesn't change the semantics, but
> is more descriptive of what's going on.
>
> Apply this change and enable the lint -- no functional change intended.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
> ---
> Makefile | 1 +
> rust/kernel/devres.rs | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c62bae2b107b..bb15b86182a3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -477,6 +477,7 @@ export rust_common_flags := --edition=2021 \
> -Wrust_2018_idioms \
> -Wunreachable_pub \
> -Wclippy::all \
> + -Wclippy::as_ptr_cast_mut \
> -Wclippy::ignored_unit_patterns \
> -Wclippy::mut_mut \
> -Wclippy::needless_bitwise_bool \
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 3a9d998ec371..598001157293 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -143,7 +143,7 @@ fn remove_action(this: &Arc<Self>) {
> bindings::devm_remove_action_nowarn(
> this.dev.as_raw(),
> Some(this.callback),
> - this.as_ptr() as _,
> + this.as_ptr().cast_mut().cast(),
> )
> };
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!`
2025-03-09 16:00 ` [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
@ 2025-03-12 14:22 ` Benno Lossin
2025-03-12 15:13 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 14:22 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> Avoid casting the input pointer to `*const _`, allowing the output
> pointer to be `*mut` if the input is `*mut`. This allows a number of
> `*const` to `*mut` conversions to be removed at the cost of slightly
> worse ergonomics when the macro is used with a reference rather than a
> pointer; the only example of this was in the macro's own doctest.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
One tiny nit below, but even without that:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/lib.rs | 5 ++---
> rust/kernel/pci.rs | 2 +-
> rust/kernel/platform.rs | 2 +-
> rust/kernel/rbtree.rs | 23 ++++++++++-------------
> 4 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7697c60b2d1a..9cd6b6864739 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -187,7 +187,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
> /// }
> ///
> /// let test = Test { a: 10, b: 20 };
> -/// let b_ptr = &test.b;
> +/// let b_ptr: *const _ = &test.b;
You could also use `&raw test.b` to get a pointer instead of relying on
the pointer coercion. That syntax is stable since 1.82.0, so older
compilers would need to enable the `raw_ref_op` feature.
I created an orthogonal good-first-issue for changing uses of
`addr_of[_mut]!` to `&raw [mut]`, so maybe that can also be done there:
https://github.com/Rust-for-Linux/linux/issues/1148
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint
2025-03-09 16:00 ` [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
@ 2025-03-12 14:39 ` Benno Lossin
2025-03-12 15:18 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 14:39 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
>
>> Though `as` casts between raw pointers are not terrible,
>> `pointer::cast` is safer because it cannot accidentally change the
>> pointer's mutability, nor cast the pointer to other types like `usize`.
>
> There are a few classes of changes required:
> - Modules generated by bindgen are marked
> `#[allow(clippy::ptr_as_ptr)]`.
> - Inferred casts (` as _`) are replaced with `.cast()`.
> - Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
> - Multistep casts from references (` as *const _ as *const T`) are
> replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
Similarly to the other patch, this could be `let x = &raw x;`. (but it's
fine to leave it as-is for now, we can also make that a
good-first-issue.)
> according to the previous rules. The intermediate `let` binding is
> required because `(x as *const _).cast::<T>()` results in inference
> failure.
> - Native literal C strings are replaced with `c_str!().as_char_ptr()`.
>
> Apply these changes and enable the lint -- no functional change
> intended.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
> ---
> Makefile | 1 +
> rust/bindings/lib.rs | 1 +
> rust/kernel/alloc/allocator_test.rs | 2 +-
> rust/kernel/alloc/kvec.rs | 4 ++--
> rust/kernel/device.rs | 5 +++--
> rust/kernel/devres.rs | 2 +-
> rust/kernel/error.rs | 2 +-
> rust/kernel/fs/file.rs | 2 +-
> rust/kernel/kunit.rs | 15 +++++++--------
> rust/kernel/list/impl_list_item_mod.rs | 2 +-
> rust/kernel/pci.rs | 2 +-
> rust/kernel/platform.rs | 4 +++-
> rust/kernel/print.rs | 11 +++++------
> rust/kernel/seq_file.rs | 3 ++-
> rust/kernel/str.rs | 2 +-
> rust/kernel/sync/poll.rs | 2 +-
> rust/kernel/workqueue.rs | 10 +++++-----
> rust/uapi/lib.rs | 1 +
> 18 files changed, 38 insertions(+), 33 deletions(-)
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint
2025-03-09 16:00 ` [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
@ 2025-03-12 14:40 ` Benno Lossin
0 siblings, 0 replies; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 14:40 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> In Rust 1.72.0, Clippy introduced the `ptr_cast_constness` lint [1]:
>
>> Though `as` casts between raw pointers are not terrible,
>> `pointer::cast_mut` and `pointer::cast_const` are safer because they
>> cannot accidentally cast the pointer to another type.
>
> There are only 2 affected sites:
> - `*mut T as *const U as *mut U` becomes `(*mut T).cast()`
> - `&self as *const Self as *mut Self` becomes a reference-to-pointer
> coercion + `(*const Self).cast()`.
>
> Apply these changes and enable the lint -- no functional change
> intended.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
> ---
> Makefile | 1 +
> rust/kernel/block/mq/request.rs | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-09 16:00 ` [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint Tamir Duberstein
@ 2025-03-12 15:05 ` Benno Lossin
2025-03-12 15:35 ` Tamir Duberstein
2025-03-12 15:41 ` Miguel Ojeda
0 siblings, 2 replies; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 15:05 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 598001157293..20159b7c9293 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -45,7 +45,7 @@ struct DevresInner<T> {
> /// # Example
> ///
> /// ```no_run
> -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
> +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
> /// # use core::ops::Deref;
> ///
> /// // See also [`pci::Bar`] for a real example.
> @@ -59,19 +59,19 @@ struct DevresInner<T> {
> /// unsafe fn new(paddr: usize) -> Result<Self>{
> /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> /// // valid for `ioremap`.
> -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
> +/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
The argument of `ioremap` is defined as `resource_size_t` which
ultimately maps to `u64` on 64 bit systems and `u32` on 32 bit ones. I
don't think that we should have code like this... Is there another
option?
Maybe Gary knows something here, do we have a type that represents that
better?
> /// if addr.is_null() {
> /// return Err(ENOMEM);
> /// }
> ///
> -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
> +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
This should be `addr.addr()` (requires `strict_provenance` on Rust 1.83
& before).
(I am assuming that we're never casting the usize back to a pointer,
since otherwise this change would introduce UB)
> /// }
> /// }
> ///
> /// impl<const SIZE: usize> Drop for IoMem<SIZE> {
> /// fn drop(&mut self) {
> /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
> -/// unsafe { bindings::iounmap(self.0.addr() as _); };
> +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
Can't this be a `.cast::<c_void>()`?
> /// }
> /// }
> ///
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 8654d52b0bb9..eb8fa52f08ba 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
> /// Returns the error encoded as a pointer.
> pub fn to_ptr<T>(self) -> *mut T {
> // SAFETY: `self.0` is a valid error due to its invariant.
> - unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
> + unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
Can't this be a `.into()`?
> }
>
> /// Returns a string representing the error, if one exists.
> @@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
> let addr = self.io_addr_assert::<$type_name>(offset);
>
> // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> - unsafe { bindings::$name(addr as _) }
> + unsafe { bindings::$name(addr as *const c_void) }
Also here, is `.cast::<c_void>()` enough? (and below)
> }
>
> /// Read IO data from a given offset.
> diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> index 04f2d8ef29cb..40d1bd13682c 100644
> --- a/rust/kernel/of.rs
> +++ b/rust/kernel/of.rs
> @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
>
> fn index(&self) -> usize {
> - self.0.data as _
> + self.0.data as usize
This should also be `self.0.data.addr()`.
> }
> }
>
> @@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
> // SAFETY: FFI type is valid to be zero-initialized.
> let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
>
> - // TODO: Use `clone_from_slice` once the corresponding types do match.
> + // TODO: Use `copy_from_slice` once stabilized for `const`.
This feature has just been stabilized (5 days ago!):
https://github.com/rust-lang/rust/issues/131415
@Miguel: Do we already have a target Rust version for dropping the
`RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature
now, since it will be stable by the time we bump the minimum version.
(not in this patch [series] though)
> let mut i = 0;
> while i < src.len() {
> - of.compatible[i] = src[i] as _;
> + of.compatible[i] = src[i];
> i += 1;
> }
> @@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
> // `ioptr` is valid by the safety requirements.
> // `num` is valid by the safety requirements.
> unsafe {
> - bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
> + bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
Again, probably castable.
> bindings::pci_release_region(pdev.as_raw(), num);
> }
> }
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 6a1a982b946d..0b80a119d5f0 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -692,9 +692,9 @@ fn new() -> Self {
> pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> // INVARIANT: The safety requirements guarantee the type invariants.
> Self {
> - beg: pos as _,
> - pos: pos as _,
> - end: end as _,
> + beg: pos as usize,
> + pos: pos as usize,
> + end: end as usize,
I would prefer if we use `pos.expose_provenance()` (also for `end`)
here.
> }
> }
>
> @@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
> ///
> /// N.B. It may point to invalid memory.
> pub(crate) fn pos(&self) -> *mut u8 {
> - self.pos as _
> + self.pos as *mut u8
This should then also be `with_exposed_provenance(self.pos)`
---
Cheers,
Benno
> }
>
> /// Returns the number of bytes written to the formatter.
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 8ff54105be3f..d03f3440cb5a 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -198,7 +198,7 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
> unsafe {
> w.__enqueue(move |work_ptr| {
> bindings::queue_work_on(
> - bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
> + bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32,
> queue_ptr,
> work_ptr,
> )
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 0/5] rust: reduce pointer casts, enable related lints
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (4 preceding siblings ...)
2025-03-09 16:00 ` [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint Tamir Duberstein
@ 2025-03-12 15:07 ` Benno Lossin
2025-03-12 15:14 ` Tamir Duberstein
5 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 15:07 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
Hi Tamir,
On Sun Mar 9, 2025 at 5:00 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].
>
> Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1]
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Thanks for this series! Did you encounter any instances of `$val as $ty`
where you couldn't convert them due to unsizing? I remember that we had
some cases back then (maybe Alice remembers them too?). If not then no
worries :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!`
2025-03-12 14:22 ` Benno Lossin
@ 2025-03-12 15:13 ` Tamir Duberstein
0 siblings, 0 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 15:13 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 10:22 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> > Avoid casting the input pointer to `*const _`, allowing the output
> > pointer to be `*mut` if the input is `*mut`. This allows a number of
> > `*const` to `*mut` conversions to be removed at the cost of slightly
> > worse ergonomics when the macro is used with a reference rather than a
> > pointer; the only example of this was in the macro's own doctest.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> One tiny nit below, but even without that:
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
>
> > ---
> > rust/kernel/lib.rs | 5 ++---
> > rust/kernel/pci.rs | 2 +-
> > rust/kernel/platform.rs | 2 +-
> > rust/kernel/rbtree.rs | 23 ++++++++++-------------
> > 4 files changed, 14 insertions(+), 18 deletions(-)
> >
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 7697c60b2d1a..9cd6b6864739 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -187,7 +187,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
> > /// }
> > ///
> > /// let test = Test { a: 10, b: 20 };
> > -/// let b_ptr = &test.b;
> > +/// let b_ptr: *const _ = &test.b;
>
> You could also use `&raw test.b` to get a pointer instead of relying on
> the pointer coercion. That syntax is stable since 1.82.0, so older
> compilers would need to enable the `raw_ref_op` feature.
>
> I created an orthogonal good-first-issue for changing uses of
> `addr_of[_mut]!` to `&raw [mut]`, so maybe that can also be done there:
>
> https://github.com/Rust-for-Linux/linux/issues/1148
Thanks for doing that! Yeah, I think moving to that syntax would be
good but as you say requires enabling the feature and/or bumping the
rust version, so it can't be done here directly.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 0/5] rust: reduce pointer casts, enable related lints
2025-03-12 15:07 ` [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Benno Lossin
@ 2025-03-12 15:14 ` Tamir Duberstein
0 siblings, 0 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 15:14 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 11:07 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Hi Tamir,
>
> On Sun Mar 9, 2025 at 5:00 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].
> >
> > Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1]
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> Thanks for this series! Did you encounter any instances of `$val as $ty`
> where you couldn't convert them due to unsizing? I remember that we had
> some cases back then (maybe Alice remembers them too?). If not then no
> worries :)
I didn't :)
Thank you for the reviews!
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint
2025-03-12 14:39 ` Benno Lossin
@ 2025-03-12 15:18 ` Tamir Duberstein
2025-03-12 15:25 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 15:18 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 10:40 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> > In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
> >
> >> Though `as` casts between raw pointers are not terrible,
> >> `pointer::cast` is safer because it cannot accidentally change the
> >> pointer's mutability, nor cast the pointer to other types like `usize`.
> >
> > There are a few classes of changes required:
> > - Modules generated by bindgen are marked
> > `#[allow(clippy::ptr_as_ptr)]`.
> > - Inferred casts (` as _`) are replaced with `.cast()`.
> > - Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
> > - Multistep casts from references (` as *const _ as *const T`) are
> > replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
>
> Similarly to the other patch, this could be `let x = &raw x;`. (but it's
> fine to leave it as-is for now, we can also make that a
> good-first-issue.)
Yeah, same as the other patch; we can't directly do that here without
introducing some compiler infra or bumping MSRV.
> > according to the previous rules. The intermediate `let` binding is
> > required because `(x as *const _).cast::<T>()` results in inference
> > failure.
> > - Native literal C strings are replaced with `c_str!().as_char_ptr()`.
> >
> > Apply these changes and enable the lint -- no functional change
> > intended.
> >
> > Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1]
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Thanks!
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint
2025-03-12 15:18 ` Tamir Duberstein
@ 2025-03-12 15:25 ` Benno Lossin
0 siblings, 0 replies; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 15:25 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 4:18 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 10:40 AM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
>> > In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
>> >
>> >> Though `as` casts between raw pointers are not terrible,
>> >> `pointer::cast` is safer because it cannot accidentally change the
>> >> pointer's mutability, nor cast the pointer to other types like `usize`.
>> >
>> > There are a few classes of changes required:
>> > - Modules generated by bindgen are marked
>> > `#[allow(clippy::ptr_as_ptr)]`.
>> > - Inferred casts (` as _`) are replaced with `.cast()`.
>> > - Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
>> > - Multistep casts from references (` as *const _ as *const T`) are
>> > replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
>>
>> Similarly to the other patch, this could be `let x = &raw x;`. (but it's
>> fine to leave it as-is for now, we can also make that a
>> good-first-issue.)
>
> Yeah, same as the other patch; we can't directly do that here without
> introducing some compiler infra or bumping MSRV.
I think it's fine enabling the feature for this patch (or in a prior
one) if you want to do the work. But someone already took up the issue I
created, so maybe it's best to let them handle it.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 15:05 ` Benno Lossin
@ 2025-03-12 15:35 ` Tamir Duberstein
2025-03-12 15:49 ` Miguel Ojeda
2025-03-12 17:05 ` Benno Lossin
2025-03-12 15:41 ` Miguel Ojeda
1 sibling, 2 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 15:35 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 11:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> > diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> > index 598001157293..20159b7c9293 100644
> > --- a/rust/kernel/devres.rs
> > +++ b/rust/kernel/devres.rs
> > @@ -45,7 +45,7 @@ struct DevresInner<T> {
> > /// # Example
> > ///
> > /// ```no_run
> > -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
> > +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
> > /// # use core::ops::Deref;
> > ///
> > /// // See also [`pci::Bar`] for a real example.
> > @@ -59,19 +59,19 @@ struct DevresInner<T> {
> > /// unsafe fn new(paddr: usize) -> Result<Self>{
> > /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> > /// // valid for `ioremap`.
> > -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
> > +/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
>
> The argument of `ioremap` is defined as `resource_size_t` which
> ultimately maps to `u64` on 64 bit systems and `u32` on 32 bit ones. I
> don't think that we should have code like this... Is there another
> option?
>
> Maybe Gary knows something here, do we have a type that represents that
> better?
Ah yeah the problem is that this type is an alias rather than a
newtype. How do you feel about `as bindings::phys_addr_t`?
> > /// if addr.is_null() {
> > /// return Err(ENOMEM);
> > /// }
> > ///
> > -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
> > +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
>
> This should be `addr.addr()` (requires `strict_provenance` on Rust 1.83
> & before).
>
> (I am assuming that we're never casting the usize back to a pointer,
> since otherwise this change would introduce UB)
Yeah, we don't have strict provenance APIs (and we can't introduce
them without compiler tooling or bumping MSRV). I'm not sure if we are
casting back to a pointer, but either way this change doesn't change
the semantics - it is only spelling out the type.
> > /// }
> > /// }
> > ///
> > /// impl<const SIZE: usize> Drop for IoMem<SIZE> {
> > /// fn drop(&mut self) {
> > /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
> > -/// unsafe { bindings::iounmap(self.0.addr() as _); };
> > +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
>
> Can't this be a `.cast::<c_void>()`?
This is an integer-to-pointer cast. `addr` returns `usize`:
impl<const SIZE: usize> IoRaw<SIZE> {
[...]
/// Returns the base address of the MMIO region.
#[inline]
pub fn addr(&self) -> usize {
self.addr
}
[...]
}
>
> > /// }
> > /// }
> > ///
>
> > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> > index 8654d52b0bb9..eb8fa52f08ba 100644
> > --- a/rust/kernel/error.rs
> > +++ b/rust/kernel/error.rs
> > @@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
> > /// Returns the error encoded as a pointer.
> > pub fn to_ptr<T>(self) -> *mut T {
> > // SAFETY: `self.0` is a valid error due to its invariant.
> > - unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
> > + unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
>
> Can't this be a `.into()`?
error[E0277]: the trait bound `isize: core::convert::From<i32>` is not satisfied
--> ../rust/kernel/error.rs:155:49
|
155 | unsafe { bindings::ERR_PTR(self.0.get().into()).cast() }
| ^^^^ the trait
`core::convert::From<i32>` is not implemented for `isize`
>
> > }
> >
> > /// Returns a string representing the error, if one exists.
>
> > @@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
> > let addr = self.io_addr_assert::<$type_name>(offset);
> >
> > // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> > - unsafe { bindings::$name(addr as _) }
> > + unsafe { bindings::$name(addr as *const c_void) }
>
> Also here, is `.cast::<c_void>()` enough? (and below)
It's an integer-to-pointer cast. In the same `impl<const SIZE: usize>
IoRaw<SIZE>` as above:
fn io_addr_assert<U>(&self, offset: usize) -> usize {
build_assert!(Self::offset_valid::<U>(offset, SIZE));
self.addr() + offset
}
>
> > }
> >
> > /// Read IO data from a given offset.
>
> > diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> > index 04f2d8ef29cb..40d1bd13682c 100644
> > --- a/rust/kernel/of.rs
> > +++ b/rust/kernel/of.rs
> > @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> > const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
> >
> > fn index(&self) -> usize {
> > - self.0.data as _
> > + self.0.data as usize
>
> This should also be `self.0.data.addr()`.
Can't do it without strict_provenance.
>
> > }
> > }
> >
> > @@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
> > // SAFETY: FFI type is valid to be zero-initialized.
> > let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
> >
> > - // TODO: Use `clone_from_slice` once the corresponding types do match.
> > + // TODO: Use `copy_from_slice` once stabilized for `const`.
>
> This feature has just been stabilized (5 days ago!):
>
> https://github.com/rust-lang/rust/issues/131415
Yep! I know :)
> @Miguel: Do we already have a target Rust version for dropping the
> `RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature
> now, since it will be stable by the time we bump the minimum version.
> (not in this patch [series] though)
>
> > let mut i = 0;
> > while i < src.len() {
> > - of.compatible[i] = src[i] as _;
> > + of.compatible[i] = src[i];
> > i += 1;
> > }
>
> > @@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
> > // `ioptr` is valid by the safety requirements.
> > // `num` is valid by the safety requirements.
> > unsafe {
> > - bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
> > + bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
>
> Again, probably castable.
How? `ioptr` is a `usize` (you can see the prototype).
>
> > bindings::pci_release_region(pdev.as_raw(), num);
> > }
> > }
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index 6a1a982b946d..0b80a119d5f0 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -692,9 +692,9 @@ fn new() -> Self {
> > pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> > // INVARIANT: The safety requirements guarantee the type invariants.
> > Self {
> > - beg: pos as _,
> > - pos: pos as _,
> > - end: end as _,
> > + beg: pos as usize,
> > + pos: pos as usize,
> > + end: end as usize,
>
> I would prefer if we use `pos.expose_provenance()` (also for `end`)
> here.
Me too! But we can't until we actually start using strict provenance.
>
> > }
> > }
> >
> > @@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
> > ///
> > /// N.B. It may point to invalid memory.
> > pub(crate) fn pos(&self) -> *mut u8 {
> > - self.pos as _
> > + self.pos as *mut u8
>
> This should then also be `with_exposed_provenance(self.pos)`
Same as other instances of strict provenance.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 15:05 ` Benno Lossin
2025-03-12 15:35 ` Tamir Duberstein
@ 2025-03-12 15:41 ` Miguel Ojeda
1 sibling, 0 replies; 38+ messages in thread
From: Miguel Ojeda @ 2025-03-12 15:41 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed, Mar 12, 2025 at 4:05 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> This feature has just been stabilized (5 days ago!):
>
> https://github.com/rust-lang/rust/issues/131415
>
> @Miguel: Do we already have a target Rust version for dropping the
> `RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature
> now, since it will be stable by the time we bump the minimum version.
> (not in this patch [series] though)
We don't (in any case, while we will not use languages unstable
features soon, we will still need tooling features).
So please feel free to use it, but it seems it is only unstably const
since 1.85.0, no?
Cheers,
Miguel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 15:35 ` Tamir Duberstein
@ 2025-03-12 15:49 ` Miguel Ojeda
2025-03-12 17:05 ` Benno Lossin
1 sibling, 0 replies; 38+ messages in thread
From: Miguel Ojeda @ 2025-03-12 15:49 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed, Mar 12, 2025 at 4:36 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Yeah, we don't have strict provenance APIs (and we can't introduce
> them without compiler tooling or bumping MSRV). I'm not sure if we are
The strict provenance APIs were added a long time ago (1.61) -- in
fact we briefly discussed doing so back then (we started before that,
with 1.58).
So unless we need some detail that changed recently (i.e. since 1.78),
we should be able to use them, and it should be fairly safe since they
are stable now (1.84).
Cheers,
Miguel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 15:35 ` Tamir Duberstein
2025-03-12 15:49 ` Miguel Ojeda
@ 2025-03-12 17:05 ` Benno Lossin
2025-03-12 18:01 ` Tamir Duberstein
1 sibling, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 17:05 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 4:35 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 11:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
>> > diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
>> > index 598001157293..20159b7c9293 100644
>> > --- a/rust/kernel/devres.rs
>> > +++ b/rust/kernel/devres.rs
>> > @@ -45,7 +45,7 @@ struct DevresInner<T> {
>> > /// # Example
>> > ///
>> > /// ```no_run
>> > -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
>> > +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
>> > /// # use core::ops::Deref;
>> > ///
>> > /// // See also [`pci::Bar`] for a real example.
>> > @@ -59,19 +59,19 @@ struct DevresInner<T> {
>> > /// unsafe fn new(paddr: usize) -> Result<Self>{
>> > /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
>> > /// // valid for `ioremap`.
>> > -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
>> > +/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
>>
>> The argument of `ioremap` is defined as `resource_size_t` which
>> ultimately maps to `u64` on 64 bit systems and `u32` on 32 bit ones. I
>> don't think that we should have code like this... Is there another
>> option?
>>
>> Maybe Gary knows something here, do we have a type that represents that
>> better?
>
> Ah yeah the problem is that this type is an alias rather than a
> newtype. How do you feel about `as bindings::phys_addr_t`?
Yeah that's better.
>> > /// if addr.is_null() {
>> > /// return Err(ENOMEM);
>> > /// }
>> > ///
>> > -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
>> > +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
>>
>> This should be `addr.addr()` (requires `strict_provenance` on Rust 1.83
>> & before).
>>
>> (I am assuming that we're never casting the usize back to a pointer,
>> since otherwise this change would introduce UB)
>
> Yeah, we don't have strict provenance APIs (and we can't introduce
> them without compiler tooling or bumping MSRV). I'm not sure if we are
> casting back to a pointer, but either way this change doesn't change
> the semantics - it is only spelling out the type.
It's fine to enable the feature, since it's stable in a newer version of
the compiler.
>> > /// }
>> > /// }
>> > ///
>> > /// impl<const SIZE: usize> Drop for IoMem<SIZE> {
>> > /// fn drop(&mut self) {
>> > /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
>> > -/// unsafe { bindings::iounmap(self.0.addr() as _); };
>> > +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
>>
>> Can't this be a `.cast::<c_void>()`?
>
> This is an integer-to-pointer cast. `addr` returns `usize`:
Oh I missed the `*mut`... In that case, we can't use the `addr`
suggestion that I made above, instead we should use `expose_provenance`
above and `with_exposed_provenance` here.
> impl<const SIZE: usize> IoRaw<SIZE> {
> [...]
>
> /// Returns the base address of the MMIO region.
> #[inline]
> pub fn addr(&self) -> usize {
> self.addr
> }
>
> [...]
> }
>
>>
>> > /// }
>> > /// }
>> > ///
>>
>> > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
>> > index 8654d52b0bb9..eb8fa52f08ba 100644
>> > --- a/rust/kernel/error.rs
>> > +++ b/rust/kernel/error.rs
>> > @@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
>> > /// Returns the error encoded as a pointer.
>> > pub fn to_ptr<T>(self) -> *mut T {
>> > // SAFETY: `self.0` is a valid error due to its invariant.
>> > - unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
>> > + unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
>>
>> Can't this be a `.into()`?
>
> error[E0277]: the trait bound `isize: core::convert::From<i32>` is not satisfied
> --> ../rust/kernel/error.rs:155:49
> |
> 155 | unsafe { bindings::ERR_PTR(self.0.get().into()).cast() }
> | ^^^^ the trait
> `core::convert::From<i32>` is not implemented for `isize`
That's a bummer... I wonder why that doesn't exist.
>> > }
>> >
>> > /// Returns a string representing the error, if one exists.
>>
>> > @@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
>> > let addr = self.io_addr_assert::<$type_name>(offset);
>> >
>> > // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
>> > - unsafe { bindings::$name(addr as _) }
>> > + unsafe { bindings::$name(addr as *const c_void) }
>>
>> Also here, is `.cast::<c_void>()` enough? (and below)
>
> It's an integer-to-pointer cast. In the same `impl<const SIZE: usize>
> IoRaw<SIZE>` as above:
>
> fn io_addr_assert<U>(&self, offset: usize) -> usize {
> build_assert!(Self::offset_valid::<U>(offset, SIZE));
>
> self.addr() + offset
> }
I would prefer we use the strict_provenance API.
>> > }
>> >
>> > /// Read IO data from a given offset.
>>
>> > diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
>> > index 04f2d8ef29cb..40d1bd13682c 100644
>> > --- a/rust/kernel/of.rs
>> > +++ b/rust/kernel/of.rs
>> > @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
>> > const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
>> >
>> > fn index(&self) -> usize {
>> > - self.0.data as _
>> > + self.0.data as usize
>>
>> This should also be `self.0.data.addr()`.
>
> Can't do it without strict_provenance.
>
>>
>> > }
>> > }
>> >
>> > @@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
>> > // SAFETY: FFI type is valid to be zero-initialized.
>> > let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
>> >
>> > - // TODO: Use `clone_from_slice` once the corresponding types do match.
>> > + // TODO: Use `copy_from_slice` once stabilized for `const`.
>>
>> This feature has just been stabilized (5 days ago!):
>>
>> https://github.com/rust-lang/rust/issues/131415
>
> Yep! I know :)
>
>> @Miguel: Do we already have a target Rust version for dropping the
>> `RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature
>> now, since it will be stable by the time we bump the minimum version.
>> (not in this patch [series] though)
>>
>> > let mut i = 0;
>> > while i < src.len() {
>> > - of.compatible[i] = src[i] as _;
>> > + of.compatible[i] = src[i];
>> > i += 1;
>> > }
>>
>> > @@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
>> > // `ioptr` is valid by the safety requirements.
>> > // `num` is valid by the safety requirements.
>> > unsafe {
>> > - bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
>> > + bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
>>
>> Again, probably castable.
>
> How? `ioptr` is a `usize` (you can see the prototype).
Sorry, I missed all the `*mut`/`*const` prefixes here.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 17:05 ` Benno Lossin
@ 2025-03-12 18:01 ` Tamir Duberstein
2025-03-12 19:10 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 18:01 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 1:05 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 4:35 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 11:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
> >> > diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> >> > index 598001157293..20159b7c9293 100644
> >> > --- a/rust/kernel/devres.rs
> >> > +++ b/rust/kernel/devres.rs
> >> > @@ -45,7 +45,7 @@ struct DevresInner<T> {
> >> > /// # Example
> >> > ///
> >> > /// ```no_run
> >> > -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
> >> > +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
> >> > /// # use core::ops::Deref;
> >> > ///
> >> > /// // See also [`pci::Bar`] for a real example.
> >> > @@ -59,19 +59,19 @@ struct DevresInner<T> {
> >> > /// unsafe fn new(paddr: usize) -> Result<Self>{
> >> > /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> >> > /// // valid for `ioremap`.
> >> > -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
> >> > +/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
> >>
> >> The argument of `ioremap` is defined as `resource_size_t` which
> >> ultimately maps to `u64` on 64 bit systems and `u32` on 32 bit ones. I
> >> don't think that we should have code like this... Is there another
> >> option?
> >>
> >> Maybe Gary knows something here, do we have a type that represents that
> >> better?
> >
> > Ah yeah the problem is that this type is an alias rather than a
> > newtype. How do you feel about `as bindings::phys_addr_t`?
>
> Yeah that's better.
>
> >> > /// if addr.is_null() {
> >> > /// return Err(ENOMEM);
> >> > /// }
> >> > ///
> >> > -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
> >> > +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
> >>
> >> This should be `addr.addr()` (requires `strict_provenance` on Rust 1.83
> >> & before).
> >>
> >> (I am assuming that we're never casting the usize back to a pointer,
> >> since otherwise this change would introduce UB)
> >
> > Yeah, we don't have strict provenance APIs (and we can't introduce
> > them without compiler tooling or bumping MSRV). I'm not sure if we are
> > casting back to a pointer, but either way this change doesn't change
> > the semantics - it is only spelling out the type.
>
> It's fine to enable the feature, since it's stable in a newer version of
> the compiler.
>
> >> > /// }
> >> > /// }
> >> > ///
> >> > /// impl<const SIZE: usize> Drop for IoMem<SIZE> {
> >> > /// fn drop(&mut self) {
> >> > /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
> >> > -/// unsafe { bindings::iounmap(self.0.addr() as _); };
> >> > +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
> >>
> >> Can't this be a `.cast::<c_void>()`?
> >
> > This is an integer-to-pointer cast. `addr` returns `usize`:
>
> Oh I missed the `*mut`... In that case, we can't use the `addr`
> suggestion that I made above, instead we should use `expose_provenance`
> above and `with_exposed_provenance` here.
>
> > impl<const SIZE: usize> IoRaw<SIZE> {
> > [...]
> >
> > /// Returns the base address of the MMIO region.
> > #[inline]
> > pub fn addr(&self) -> usize {
> > self.addr
> > }
> >
> > [...]
> > }
> >
> >>
> >> > /// }
> >> > /// }
> >> > ///
> >>
> >> > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> >> > index 8654d52b0bb9..eb8fa52f08ba 100644
> >> > --- a/rust/kernel/error.rs
> >> > +++ b/rust/kernel/error.rs
> >> > @@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
> >> > /// Returns the error encoded as a pointer.
> >> > pub fn to_ptr<T>(self) -> *mut T {
> >> > // SAFETY: `self.0` is a valid error due to its invariant.
> >> > - unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
> >> > + unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
> >>
> >> Can't this be a `.into()`?
> >
> > error[E0277]: the trait bound `isize: core::convert::From<i32>` is not satisfied
> > --> ../rust/kernel/error.rs:155:49
> > |
> > 155 | unsafe { bindings::ERR_PTR(self.0.get().into()).cast() }
> > | ^^^^ the trait
> > `core::convert::From<i32>` is not implemented for `isize`
>
> That's a bummer... I wonder why that doesn't exist.
>
> >> > }
> >> >
> >> > /// Returns a string representing the error, if one exists.
> >>
> >> > @@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
> >> > let addr = self.io_addr_assert::<$type_name>(offset);
> >> >
> >> > // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> >> > - unsafe { bindings::$name(addr as _) }
> >> > + unsafe { bindings::$name(addr as *const c_void) }
> >>
> >> Also here, is `.cast::<c_void>()` enough? (and below)
> >
> > It's an integer-to-pointer cast. In the same `impl<const SIZE: usize>
> > IoRaw<SIZE>` as above:
> >
> > fn io_addr_assert<U>(&self, offset: usize) -> usize {
> > build_assert!(Self::offset_valid::<U>(offset, SIZE));
> >
> > self.addr() + offset
> > }
>
> I would prefer we use the strict_provenance API.
>
> >> > }
> >> >
> >> > /// Read IO data from a given offset.
> >>
> >> > diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> >> > index 04f2d8ef29cb..40d1bd13682c 100644
> >> > --- a/rust/kernel/of.rs
> >> > +++ b/rust/kernel/of.rs
> >> > @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> >> > const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
> >> >
> >> > fn index(&self) -> usize {
> >> > - self.0.data as _
> >> > + self.0.data as usize
> >>
> >> This should also be `self.0.data.addr()`.
> >
> > Can't do it without strict_provenance.
> >
> >>
> >> > }
> >> > }
> >> >
> >> > @@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
> >> > // SAFETY: FFI type is valid to be zero-initialized.
> >> > let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
> >> >
> >> > - // TODO: Use `clone_from_slice` once the corresponding types do match.
> >> > + // TODO: Use `copy_from_slice` once stabilized for `const`.
> >>
> >> This feature has just been stabilized (5 days ago!):
> >>
> >> https://github.com/rust-lang/rust/issues/131415
> >
> > Yep! I know :)
> >
> >> @Miguel: Do we already have a target Rust version for dropping the
> >> `RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature
> >> now, since it will be stable by the time we bump the minimum version.
> >> (not in this patch [series] though)
> >>
> >> > let mut i = 0;
> >> > while i < src.len() {
> >> > - of.compatible[i] = src[i] as _;
> >> > + of.compatible[i] = src[i];
> >> > i += 1;
> >> > }
> >>
> >> > @@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
> >> > // `ioptr` is valid by the safety requirements.
> >> > // `num` is valid by the safety requirements.
> >> > unsafe {
> >> > - bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
> >> > + bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
> >>
> >> Again, probably castable.
> >
> > How? `ioptr` is a `usize` (you can see the prototype).
>
> Sorry, I missed all the `*mut`/`*const` prefixes here.
>
> ---
> Cheers,
> Benno
>
I think all the remaining comments are about strict provenance. I buy
that this is a useful thing to do, but in the absence of automated
tools to help do it, I'm not sure it's worth it to do it for just
these things I happen to be touching rather than doing it throughout.
I couldn't find a clippy lint. Do you know of one? If not, should we
file an issue?
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 18:01 ` Tamir Duberstein
@ 2025-03-12 19:10 ` Benno Lossin
2025-03-12 19:19 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 19:10 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 7:01 PM CET, Tamir Duberstein wrote:
> I think all the remaining comments are about strict provenance. I buy
> that this is a useful thing to do, but in the absence of automated
> tools to help do it, I'm not sure it's worth it to do it for just
> these things I happen to be touching rather than doing it throughout.
>
> I couldn't find a clippy lint. Do you know of one? If not, should we
> file an issue?
A quick search gave me:
https://doc.rust-lang.org/nightly/unstable-book/language-features/strict-provenance-lints.html
The linked tracking issue is closed which seems like a mistake, since
it's not yet stabilized. I found a different issue tracking it though:
https://github.com/rust-lang/rust/issues/130351
We probably should use both in that case:
#![feature(strict_provenance_lints)]
#![warn(fuzzy_provenance_casts, lossy_provenance_casts)]
I don't want to push more work onto this series, so it's fine to leave
them in. Thus:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
We can either make this a good-first-issue, or if you also want to
tackle this, then go ahead :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 19:10 ` Benno Lossin
@ 2025-03-12 19:19 ` Tamir Duberstein
2025-03-12 19:43 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 19:19 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 3:11 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 7:01 PM CET, Tamir Duberstein wrote:
> > I think all the remaining comments are about strict provenance. I buy
> > that this is a useful thing to do, but in the absence of automated
> > tools to help do it, I'm not sure it's worth it to do it for just
> > these things I happen to be touching rather than doing it throughout.
> >
> > I couldn't find a clippy lint. Do you know of one? If not, should we
> > file an issue?
>
> A quick search gave me:
>
> https://doc.rust-lang.org/nightly/unstable-book/language-features/strict-provenance-lints.html
>
> The linked tracking issue is closed which seems like a mistake, since
> it's not yet stabilized. I found a different issue tracking it though:
>
> https://github.com/rust-lang/rust/issues/130351
>
> We probably should use both in that case:
>
> #![feature(strict_provenance_lints)]
> #![warn(fuzzy_provenance_casts, lossy_provenance_casts)]
Nice! I didn't think to check rustc lints.
> I don't want to push more work onto this series, so it's fine to leave
> them in. Thus:
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Thanks!
>
> We can either make this a good-first-issue, or if you also want to
> tackle this, then go ahead :)
I tried using the strict provenance lints locally and I think we can't
until we properly bump MSRV due to `clippy::incompatible_msrv`:
warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
this item is stable since `1.84.0`
--> ../rust/kernel/str.rs:696:22
|
696 | pos: pos.expose_provenance(),
| ^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
This is with `#![feature(strict_provenance)]`. I can file the issue
but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
path forward :)
I'll send v3 with the tags and the phys_addr_t fix shortly.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 19:19 ` Tamir Duberstein
@ 2025-03-12 19:43 ` Benno Lossin
2025-03-12 20:07 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 19:43 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
> I tried using the strict provenance lints locally and I think we can't
> until we properly bump MSRV due to `clippy::incompatible_msrv`:
>
> warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
> this item is stable since `1.84.0`
> --> ../rust/kernel/str.rs:696:22
> |
> 696 | pos: pos.expose_provenance(),
> | ^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit
> https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
Oh this is annoying...
> This is with `#![feature(strict_provenance)]`. I can file the issue
> but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
> path forward :)
I think we should be able to just `allow(clippy::incompatible_msrv)`,
since Miguel & other maintainers will test with 1.78 (or at least are
supposed to :).
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 19:43 ` Benno Lossin
@ 2025-03-12 20:07 ` Tamir Duberstein
2025-03-12 20:41 ` Tamir Duberstein
2025-03-12 20:42 ` Benno Lossin
0 siblings, 2 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 20:07 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
> > I tried using the strict provenance lints locally and I think we can't
> > until we properly bump MSRV due to `clippy::incompatible_msrv`:
> >
> > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
> > this item is stable since `1.84.0`
> > --> ../rust/kernel/str.rs:696:22
> > |
> > 696 | pos: pos.expose_provenance(),
> > | ^^^^^^^^^^^^^^^^^^^
> > |
> > = help: for further information visit
> > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
>
> Oh this is annoying...
>
> > This is with `#![feature(strict_provenance)]`. I can file the issue
> > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
> > path forward :)
>
> I think we should be able to just `allow(clippy::incompatible_msrv)`,
> since Miguel & other maintainers will test with 1.78 (or at least are
> supposed to :).
Alright, you've sniped me. This is coming in v3.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 20:07 ` Tamir Duberstein
@ 2025-03-12 20:41 ` Tamir Duberstein
2025-03-12 21:01 ` Benno Lossin
2025-03-12 20:42 ` Benno Lossin
1 sibling, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 20:41 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 4:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
> > > I tried using the strict provenance lints locally and I think we can't
> > > until we properly bump MSRV due to `clippy::incompatible_msrv`:
> > >
> > > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
> > > this item is stable since `1.84.0`
> > > --> ../rust/kernel/str.rs:696:22
> > > |
> > > 696 | pos: pos.expose_provenance(),
> > > | ^^^^^^^^^^^^^^^^^^^
> > > |
> > > = help: for further information visit
> > > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
> >
> > Oh this is annoying...
> >
> > > This is with `#![feature(strict_provenance)]`. I can file the issue
> > > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
> > > path forward :)
> >
> > I think we should be able to just `allow(clippy::incompatible_msrv)`,
> > since Miguel & other maintainers will test with 1.78 (or at least are
> > supposed to :).
>
> Alright, you've sniped me. This is coming in v3.
I just realized I only covered the kernel crate. In order to cover all
Rust code, I need to move the lints and the features out to the root
Makefile. I tried something like this:
> diff --git a/Makefile b/Makefile
> index 2af40bfed9ce..10af1e44370b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -466,13 +466,21 @@ KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
> KBUILD_USERCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS)
> KBUILD_USERLDFLAGS := $(USERLDFLAGS)
>
> +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
> +#
> +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE = $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> +
> # These flags apply to all Rust code in the tree, including the kernel and
> # host programs.
> export rust_common_flags := --edition=2021 \
> -Zbinary_dep_depinfo=y \
> + -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" \
> -Astable_features \
> -Dnon_ascii_idents \
> -Dunsafe_op_in_unsafe_fn \
> + -Wfuzzy_provenance_casts \
> + -Wlossy_provenance_casts \
> -Wmissing_docs \
> -Wrust_2018_idioms \
> -Wunreachable_pub \
> diff --git a/rust/Makefile b/rust/Makefile
> index ea3849eb78f6..d7d5be741245 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -435,8 +435,10 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
> # symbol versions generated from Rust objects.
> $(obj)/exports.o: private skip_gendwarfksyms = 1
>
> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> +
> $(obj)/core.o: private skip_clippy = 1
> -$(obj)/core.o: private skip_flags = -Wunreachable_pub
> +$(obj)/core.o: private skip_flags = -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" -Wunreachable_pub -Wfuzzy_provenance_casts -Wlossy_provenance_casts
> $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
> $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
> $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
but this doesn't work because
`CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE` is not yet defined when I
read it in the root Makefile. I can read it lower down and then append
the feature flag to `KBUILD_RUSTFLAGS` but by then the rustdoc flags
have been copied from `rust_common_flags` and so rustdoc doesn't get
the feature flag, resulting in unknown lint warnings in rustdoc and
kunit tests.
Any ideas?
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 20:07 ` Tamir Duberstein
2025-03-12 20:41 ` Tamir Duberstein
@ 2025-03-12 20:42 ` Benno Lossin
1 sibling, 0 replies; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 20:42 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 9:07 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
>> > I tried using the strict provenance lints locally and I think we can't
>> > until we properly bump MSRV due to `clippy::incompatible_msrv`:
>> >
>> > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
>> > this item is stable since `1.84.0`
>> > --> ../rust/kernel/str.rs:696:22
>> > |
>> > 696 | pos: pos.expose_provenance(),
>> > | ^^^^^^^^^^^^^^^^^^^
>> > |
>> > = help: for further information visit
>> > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
>>
>> Oh this is annoying...
>>
>> > This is with `#![feature(strict_provenance)]`. I can file the issue
>> > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
>> > path forward :)
>>
>> I think we should be able to just `allow(clippy::incompatible_msrv)`,
>> since Miguel & other maintainers will test with 1.78 (or at least are
>> supposed to :).
>
> Alright, you've sniped me.
Sorry about that :)
> This is coming in v3.
Thanks a lot!
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 20:41 ` Tamir Duberstein
@ 2025-03-12 21:01 ` Benno Lossin
2025-03-12 21:04 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 21:01 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 9:41 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 4:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>>
>> On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >
>> > On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
>> > > I tried using the strict provenance lints locally and I think we can't
>> > > until we properly bump MSRV due to `clippy::incompatible_msrv`:
>> > >
>> > > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
>> > > this item is stable since `1.84.0`
>> > > --> ../rust/kernel/str.rs:696:22
>> > > |
>> > > 696 | pos: pos.expose_provenance(),
>> > > | ^^^^^^^^^^^^^^^^^^^
>> > > |
>> > > = help: for further information visit
>> > > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
>> >
>> > Oh this is annoying...
>> >
>> > > This is with `#![feature(strict_provenance)]`. I can file the issue
>> > > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
>> > > path forward :)
>> >
>> > I think we should be able to just `allow(clippy::incompatible_msrv)`,
>> > since Miguel & other maintainers will test with 1.78 (or at least are
>> > supposed to :).
>>
>> Alright, you've sniped me. This is coming in v3.
>
> I just realized I only covered the kernel crate. In order to cover all
> Rust code, I need to move the lints and the features out to the root
> Makefile. I tried something like this:
>
>> diff --git a/Makefile b/Makefile
>> index 2af40bfed9ce..10af1e44370b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -466,13 +466,21 @@ KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
>> KBUILD_USERCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS)
>> KBUILD_USERLDFLAGS := $(USERLDFLAGS)
>>
>> +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
>> +#
>> +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
>> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE = $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
>> +
>> # These flags apply to all Rust code in the tree, including the kernel and
>> # host programs.
>> export rust_common_flags := --edition=2021 \
>> -Zbinary_dep_depinfo=y \
>> + -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" \
>> -Astable_features \
>> -Dnon_ascii_idents \
>> -Dunsafe_op_in_unsafe_fn \
>> + -Wfuzzy_provenance_casts \
>> + -Wlossy_provenance_casts \
>> -Wmissing_docs \
>> -Wrust_2018_idioms \
>> -Wunreachable_pub \
>> diff --git a/rust/Makefile b/rust/Makefile
>> index ea3849eb78f6..d7d5be741245 100644
>> --- a/rust/Makefile
>> +++ b/rust/Makefile
>> @@ -435,8 +435,10 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
>> # symbol versions generated from Rust objects.
>> $(obj)/exports.o: private skip_gendwarfksyms = 1
>>
>> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
>> +
>> $(obj)/core.o: private skip_clippy = 1
>> -$(obj)/core.o: private skip_flags = -Wunreachable_pub
>> +$(obj)/core.o: private skip_flags = -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" -Wunreachable_pub -Wfuzzy_provenance_casts -Wlossy_provenance_casts
>> $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
>> $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
>> $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
>
> but this doesn't work because
> `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE` is not yet defined when I
> read it in the root Makefile. I can read it lower down and then append
> the feature flag to `KBUILD_RUSTFLAGS` but by then the rustdoc flags
> have been copied from `rust_common_flags` and so rustdoc doesn't get
> the feature flag, resulting in unknown lint warnings in rustdoc and
> kunit tests.
>
> Any ideas?
Always enable the features, we have `allow(stable_features)` for this
reason (then you don't have to do this dance with checking if it's
already stable or not :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 21:01 ` Benno Lossin
@ 2025-03-12 21:04 ` Tamir Duberstein
2025-03-12 21:10 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 21:04 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 9:41 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 4:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >>
> >> On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >
> >> > On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
> >> > > I tried using the strict provenance lints locally and I think we can't
> >> > > until we properly bump MSRV due to `clippy::incompatible_msrv`:
> >> > >
> >> > > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
> >> > > this item is stable since `1.84.0`
> >> > > --> ../rust/kernel/str.rs:696:22
> >> > > |
> >> > > 696 | pos: pos.expose_provenance(),
> >> > > | ^^^^^^^^^^^^^^^^^^^
> >> > > |
> >> > > = help: for further information visit
> >> > > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
> >> >
> >> > Oh this is annoying...
> >> >
> >> > > This is with `#![feature(strict_provenance)]`. I can file the issue
> >> > > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
> >> > > path forward :)
> >> >
> >> > I think we should be able to just `allow(clippy::incompatible_msrv)`,
> >> > since Miguel & other maintainers will test with 1.78 (or at least are
> >> > supposed to :).
> >>
> >> Alright, you've sniped me. This is coming in v3.
> >
> > I just realized I only covered the kernel crate. In order to cover all
> > Rust code, I need to move the lints and the features out to the root
> > Makefile. I tried something like this:
> >
> >> diff --git a/Makefile b/Makefile
> >> index 2af40bfed9ce..10af1e44370b 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -466,13 +466,21 @@ KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
> >> KBUILD_USERCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS)
> >> KBUILD_USERLDFLAGS := $(USERLDFLAGS)
> >>
> >> +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
> >> +#
> >> +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
> >> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE = $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> >> +
> >> # These flags apply to all Rust code in the tree, including the kernel and
> >> # host programs.
> >> export rust_common_flags := --edition=2021 \
> >> -Zbinary_dep_depinfo=y \
> >> + -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" \
> >> -Astable_features \
> >> -Dnon_ascii_idents \
> >> -Dunsafe_op_in_unsafe_fn \
> >> + -Wfuzzy_provenance_casts \
> >> + -Wlossy_provenance_casts \
> >> -Wmissing_docs \
> >> -Wrust_2018_idioms \
> >> -Wunreachable_pub \
> >> diff --git a/rust/Makefile b/rust/Makefile
> >> index ea3849eb78f6..d7d5be741245 100644
> >> --- a/rust/Makefile
> >> +++ b/rust/Makefile
> >> @@ -435,8 +435,10 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
> >> # symbol versions generated from Rust objects.
> >> $(obj)/exports.o: private skip_gendwarfksyms = 1
> >>
> >> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> >> +
> >> $(obj)/core.o: private skip_clippy = 1
> >> -$(obj)/core.o: private skip_flags = -Wunreachable_pub
> >> +$(obj)/core.o: private skip_flags = -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" -Wunreachable_pub -Wfuzzy_provenance_casts -Wlossy_provenance_casts
> >> $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
> >> $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
> >> $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
> >
> > but this doesn't work because
> > `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE` is not yet defined when I
> > read it in the root Makefile. I can read it lower down and then append
> > the feature flag to `KBUILD_RUSTFLAGS` but by then the rustdoc flags
> > have been copied from `rust_common_flags` and so rustdoc doesn't get
> > the feature flag, resulting in unknown lint warnings in rustdoc and
> > kunit tests.
> >
> > Any ideas?
>
> Always enable the features, we have `allow(stable_features)` for this
> reason (then you don't have to do this dance with checking if it's
> already stable or not :)
It's not so simple. In rustc < 1.84.0 the lints *and* the strict
provenance APIs are behind `feature(strict_provenance)`. In rustc >=
1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
need to read the config to learn that you need to enable
`feature(strict_provenance_lints)`.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 21:04 ` Tamir Duberstein
@ 2025-03-12 21:10 ` Tamir Duberstein
2025-03-12 21:30 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 21:10 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > On Wed Mar 12, 2025 at 9:41 PM CET, Tamir Duberstein wrote:
> > > On Wed, Mar 12, 2025 at 4:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > >>
> > >> On Wed, Mar 12, 2025 at 3:43 PM Benno Lossin <benno.lossin@proton.me> wrote:
> > >> >
> > >> > On Wed Mar 12, 2025 at 8:19 PM CET, Tamir Duberstein wrote:
> > >> > > I tried using the strict provenance lints locally and I think we can't
> > >> > > until we properly bump MSRV due to `clippy::incompatible_msrv`:
> > >> > >
> > >> > > warning: current MSRV (Minimum Supported Rust Version) is `1.78.0` but
> > >> > > this item is stable since `1.84.0`
> > >> > > --> ../rust/kernel/str.rs:696:22
> > >> > > |
> > >> > > 696 | pos: pos.expose_provenance(),
> > >> > > | ^^^^^^^^^^^^^^^^^^^
> > >> > > |
> > >> > > = help: for further information visit
> > >> > > https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
> > >> >
> > >> > Oh this is annoying...
> > >> >
> > >> > > This is with `#![feature(strict_provenance)]`. I can file the issue
> > >> > > but I think it's blocked on MSRV >= 1.84.0. But maybe you know of a
> > >> > > path forward :)
> > >> >
> > >> > I think we should be able to just `allow(clippy::incompatible_msrv)`,
> > >> > since Miguel & other maintainers will test with 1.78 (or at least are
> > >> > supposed to :).
> > >>
> > >> Alright, you've sniped me. This is coming in v3.
> > >
> > > I just realized I only covered the kernel crate. In order to cover all
> > > Rust code, I need to move the lints and the features out to the root
> > > Makefile. I tried something like this:
> > >
> > >> diff --git a/Makefile b/Makefile
> > >> index 2af40bfed9ce..10af1e44370b 100644
> > >> --- a/Makefile
> > >> +++ b/Makefile
> > >> @@ -466,13 +466,21 @@ KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
> > >> KBUILD_USERCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS)
> > >> KBUILD_USERLDFLAGS := $(USERLDFLAGS)
> > >>
> > >> +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
> > >> +#
> > >> +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
> > >> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE = $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> > >> +
> > >> # These flags apply to all Rust code in the tree, including the kernel and
> > >> # host programs.
> > >> export rust_common_flags := --edition=2021 \
> > >> -Zbinary_dep_depinfo=y \
> > >> + -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" \
> > >> -Astable_features \
> > >> -Dnon_ascii_idents \
> > >> -Dunsafe_op_in_unsafe_fn \
> > >> + -Wfuzzy_provenance_casts \
> > >> + -Wlossy_provenance_casts \
> > >> -Wmissing_docs \
> > >> -Wrust_2018_idioms \
> > >> -Wunreachable_pub \
> > >> diff --git a/rust/Makefile b/rust/Makefile
> > >> index ea3849eb78f6..d7d5be741245 100644
> > >> --- a/rust/Makefile
> > >> +++ b/rust/Makefile
> > >> @@ -435,8 +435,10 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
> > >> # symbol versions generated from Rust objects.
> > >> $(obj)/exports.o: private skip_gendwarfksyms = 1
> > >>
> > >> +KBUILD_RUST_STRICT_PROVENANCE_FEATURE := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> > >> +
> > >> $(obj)/core.o: private skip_clippy = 1
> > >> -$(obj)/core.o: private skip_flags = -Wunreachable_pub
> > >> +$(obj)/core.o: private skip_flags = -Zcrate-attr="feature($(KBUILD_RUST_STRICT_PROVENANCE_FEATURE))" -Wunreachable_pub -Wfuzzy_provenance_casts -Wlossy_provenance_casts
> > >> $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
> > >> $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
> > >> $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
> > >
> > > but this doesn't work because
> > > `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE` is not yet defined when I
> > > read it in the root Makefile. I can read it lower down and then append
> > > the feature flag to `KBUILD_RUSTFLAGS` but by then the rustdoc flags
> > > have been copied from `rust_common_flags` and so rustdoc doesn't get
> > > the feature flag, resulting in unknown lint warnings in rustdoc and
> > > kunit tests.
> > >
> > > Any ideas?
> >
> > Always enable the features, we have `allow(stable_features)` for this
> > reason (then you don't have to do this dance with checking if it's
> > already stable or not :)
>
> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
> need to read the config to learn that you need to enable
> `feature(strict_provenance_lints)`.
Actually this isn't even the only problem. It seems that
`-Astable_features` doesn't affect features enabled on the command
line at all:
error[E0725]: the feature `strict_provenance` is not in the list of
allowed features
--> <crate attribute>:1:9
|
1 | feature(strict_provenance)
| ^^^^^^^^^^^^^^^^^
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 21:10 ` Tamir Duberstein
@ 2025-03-12 21:30 ` Benno Lossin
2025-03-12 22:24 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 21:30 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
>>
>> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> > Always enable the features, we have `allow(stable_features)` for this
>> > reason (then you don't have to do this dance with checking if it's
>> > already stable or not :)
>>
>> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
>> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
>> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
>> need to read the config to learn that you need to enable
>> `feature(strict_provenance_lints)`.
I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
bit of a bummer...
But I guess we could have this config option (in `init/Kconfig`):
config RUSTC_HAS_STRICT_PROVENANCE
def_bool RUSTC_VERSION >= 108400
and then do this in `lib.rs`:
#![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
> Actually this isn't even the only problem. It seems that
> `-Astable_features` doesn't affect features enabled on the command
> line at all:
>
> error[E0725]: the feature `strict_provenance` is not in the list of
> allowed features
> --> <crate attribute>:1:9
> |
> 1 | feature(strict_provenance)
> | ^^^^^^^^^^^^^^^^^
That's because you need to append the feature to `rust_allowed_features`
in `scripts/Makefile.build` (AFAIK).
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 21:30 ` Benno Lossin
@ 2025-03-12 22:24 ` Tamir Duberstein
2025-03-12 22:38 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-12 22:24 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >>
> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> > Always enable the features, we have `allow(stable_features)` for this
> >> > reason (then you don't have to do this dance with checking if it's
> >> > already stable or not :)
> >>
> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
> >> need to read the config to learn that you need to enable
> >> `feature(strict_provenance_lints)`.
>
> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
> bit of a bummer...
>
> But I guess we could have this config option (in `init/Kconfig`):
>
> config RUSTC_HAS_STRICT_PROVENANCE
> def_bool RUSTC_VERSION >= 108400
>
> and then do this in `lib.rs`:
>
> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
Yep! That's exactly what I did, but as I mentioned up-thread, the
result is that we only cover the `kernel` crate.
>
> > Actually this isn't even the only problem. It seems that
> > `-Astable_features` doesn't affect features enabled on the command
> > line at all:
> >
> > error[E0725]: the feature `strict_provenance` is not in the list of
> > allowed features
> > --> <crate attribute>:1:9
> > |
> > 1 | feature(strict_provenance)
> > | ^^^^^^^^^^^^^^^^^
>
> That's because you need to append the feature to `rust_allowed_features`
> in `scripts/Makefile.build` (AFAIK).
Thanks, that's a helpful pointer, and it solves some problems but not
all. The root Makefile contains this bit:
> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> -Zallow-features= $(HOSTRUSTFLAGS)
which means we can't use the provenance lints against these host
targets (including e.g. `rustdoc_test_gen`). We can't remove this
-Zallow-features= either because then core fails to compile.
I'm at the point where I think I need more involved help. Want to take
a look at my attempt? It's here:
https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
Thanks!
Tamir
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 22:24 ` Tamir Duberstein
@ 2025-03-12 22:38 ` Benno Lossin
2025-03-13 10:47 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-12 22:38 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
>> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> >>
>> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> > Always enable the features, we have `allow(stable_features)` for this
>> >> > reason (then you don't have to do this dance with checking if it's
>> >> > already stable or not :)
>> >>
>> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
>> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
>> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
>> >> need to read the config to learn that you need to enable
>> >> `feature(strict_provenance_lints)`.
>>
>> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
>> bit of a bummer...
>>
>> But I guess we could have this config option (in `init/Kconfig`):
>>
>> config RUSTC_HAS_STRICT_PROVENANCE
>> def_bool RUSTC_VERSION >= 108400
>>
>> and then do this in `lib.rs`:
>>
>> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
>
> Yep! That's exactly what I did, but as I mentioned up-thread, the
> result is that we only cover the `kernel` crate.
Ah I see, can't we just have the above line in the other crate roots?
>> > Actually this isn't even the only problem. It seems that
>> > `-Astable_features` doesn't affect features enabled on the command
>> > line at all:
>> >
>> > error[E0725]: the feature `strict_provenance` is not in the list of
>> > allowed features
>> > --> <crate attribute>:1:9
>> > |
>> > 1 | feature(strict_provenance)
>> > | ^^^^^^^^^^^^^^^^^
>>
>> That's because you need to append the feature to `rust_allowed_features`
>> in `scripts/Makefile.build` (AFAIK).
>
> Thanks, that's a helpful pointer, and it solves some problems but not
> all. The root Makefile contains this bit:
>
>> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
>> -Zallow-features= $(HOSTRUSTFLAGS)
>
> which means we can't use the provenance lints against these host
> targets (including e.g. `rustdoc_test_gen`). We can't remove this
> -Zallow-features= either because then core fails to compile.
>
> I'm at the point where I think I need more involved help. Want to take
> a look at my attempt? It's here:
> https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
I'll take a look tomorrow, you're testing my knowledge of the build
system a lot here :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-12 22:38 ` Benno Lossin
@ 2025-03-13 10:47 ` Tamir Duberstein
2025-03-13 14:11 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-13 10:47 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed, Mar 12, 2025 at 6:38 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
> >> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >> >>
> >> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> > Always enable the features, we have `allow(stable_features)` for this
> >> >> > reason (then you don't have to do this dance with checking if it's
> >> >> > already stable or not :)
> >> >>
> >> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
> >> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
> >> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
> >> >> need to read the config to learn that you need to enable
> >> >> `feature(strict_provenance_lints)`.
> >>
> >> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
> >> bit of a bummer...
> >>
> >> But I guess we could have this config option (in `init/Kconfig`):
> >>
> >> config RUSTC_HAS_STRICT_PROVENANCE
> >> def_bool RUSTC_VERSION >= 108400
> >>
> >> and then do this in `lib.rs`:
> >>
> >> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
> >
> > Yep! That's exactly what I did, but as I mentioned up-thread, the
> > result is that we only cover the `kernel` crate.
>
> Ah I see, can't we just have the above line in the other crate roots?
The most difficult case is doctests. You'd have to add this to every
example AFAICT.
> >> > Actually this isn't even the only problem. It seems that
> >> > `-Astable_features` doesn't affect features enabled on the command
> >> > line at all:
> >> >
> >> > error[E0725]: the feature `strict_provenance` is not in the list of
> >> > allowed features
> >> > --> <crate attribute>:1:9
> >> > |
> >> > 1 | feature(strict_provenance)
> >> > | ^^^^^^^^^^^^^^^^^
> >>
> >> That's because you need to append the feature to `rust_allowed_features`
> >> in `scripts/Makefile.build` (AFAIK).
> >
> > Thanks, that's a helpful pointer, and it solves some problems but not
> > all. The root Makefile contains this bit:
> >
> >> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> >> -Zallow-features= $(HOSTRUSTFLAGS)
> >
> > which means we can't use the provenance lints against these host
> > targets (including e.g. `rustdoc_test_gen`). We can't remove this
> > -Zallow-features= either because then core fails to compile.
> >
> > I'm at the point where I think I need more involved help. Want to take
> > a look at my attempt? It's here:
> > https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
>
> I'll take a look tomorrow, you're testing my knowledge of the build
> system a lot here :)
We're guaranteed to learn something :)
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-13 10:47 ` Tamir Duberstein
@ 2025-03-13 14:11 ` Benno Lossin
2025-03-13 17:50 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-13 14:11 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Thu Mar 13, 2025 at 11:47 AM CET, Tamir Duberstein wrote:
> On Wed, Mar 12, 2025 at 6:38 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
>> > On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >>
>> >> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
>> >> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> >> >>
>> >> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> > Always enable the features, we have `allow(stable_features)` for this
>> >> >> > reason (then you don't have to do this dance with checking if it's
>> >> >> > already stable or not :)
>> >> >>
>> >> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
>> >> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
>> >> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
>> >> >> need to read the config to learn that you need to enable
>> >> >> `feature(strict_provenance_lints)`.
>> >>
>> >> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
>> >> bit of a bummer...
>> >>
>> >> But I guess we could have this config option (in `init/Kconfig`):
>> >>
>> >> config RUSTC_HAS_STRICT_PROVENANCE
>> >> def_bool RUSTC_VERSION >= 108400
>> >>
>> >> and then do this in `lib.rs`:
>> >>
>> >> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
>> >
>> > Yep! That's exactly what I did, but as I mentioned up-thread, the
>> > result is that we only cover the `kernel` crate.
>>
>> Ah I see, can't we just have the above line in the other crate roots?
>
> The most difficult case is doctests. You'd have to add this to every
> example AFAICT.
>
>> >> > Actually this isn't even the only problem. It seems that
>> >> > `-Astable_features` doesn't affect features enabled on the command
>> >> > line at all:
>> >> >
>> >> > error[E0725]: the feature `strict_provenance` is not in the list of
>> >> > allowed features
>> >> > --> <crate attribute>:1:9
>> >> > |
>> >> > 1 | feature(strict_provenance)
>> >> > | ^^^^^^^^^^^^^^^^^
>> >>
>> >> That's because you need to append the feature to `rust_allowed_features`
>> >> in `scripts/Makefile.build` (AFAIK).
>> >
>> > Thanks, that's a helpful pointer, and it solves some problems but not
>> > all. The root Makefile contains this bit:
>> >
>> >> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
>> >> -Zallow-features= $(HOSTRUSTFLAGS)
>> >
>> > which means we can't use the provenance lints against these host
>> > targets (including e.g. `rustdoc_test_gen`). We can't remove this
>> > -Zallow-features= either because then core fails to compile.
>> >
>> > I'm at the point where I think I need more involved help. Want to take
>> > a look at my attempt? It's here:
>> > https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
With doing `allow(clippy::incompatible_msrv)`, I meant doing that
globally, not having a module to re-export the functions :)
>> I'll take a look tomorrow, you're testing my knowledge of the build
>> system a lot here :)
>
> We're guaranteed to learn something :)
Yep! I managed to get it working, but it is rather janky and
experimental. I don't think you should use this in your patch series
unless Miguel has commented on it.
Notable things in the diff below:
* the hostrustflags don't get the *provenance_casts lints (which is
correct, I think, but probably not the way I did it with filter-out)
* the crates compiler_builtins, bindings, uapi, build_error, libmacros,
ffi, etc do get them, but probably shouldn't?
---
Cheers,
Benno
diff --git a/Makefile b/Makefile
index 70bdbf2218fc..38a79337cd7b 100644
--- a/Makefile
+++ b/Makefile
@@ -473,6 +473,8 @@ export rust_common_flags := --edition=2021 \
-Astable_features \
-Dnon_ascii_idents \
-Dunsafe_op_in_unsafe_fn \
+ -Wfuzzy_provenance_casts \
+ -Wlossy_provenance_casts \
-Wmissing_docs \
-Wrust_2018_idioms \
-Wunreachable_pub \
@@ -493,7 +495,7 @@ KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
$(HOSTCFLAGS) -I $(srctree)/scripts/include
KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) \
-I $(srctree)/scripts/include
-KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
+KBUILD_HOSTRUSTFLAGS := $(filter-out -Wfuzzy_provenance_casts -Wlossy_provenance_casts,$(rust_common_flags)) -O -Cstrip=debuginfo \
-Zallow-features= $(HOSTRUSTFLAGS)
KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
diff --git a/init/Kconfig b/init/Kconfig
index d0d021b3fa3b..82e28d6f7c3f 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
config RUSTC_HAS_COERCE_POINTEE
def_bool RUSTC_VERSION >= 108400
+config RUSTC_HAS_STABLE_STRICT_PROVENANCE
+ def_bool RUSTC_VERSION >= 108400
+
config PAHOLE_VERSION
int
default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/Makefile b/rust/Makefile
index ea3849eb78f6..998b57c6e5f7 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -436,7 +436,8 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
$(obj)/exports.o: private skip_gendwarfksyms = 1
$(obj)/core.o: private skip_clippy = 1
-$(obj)/core.o: private skip_flags = -Wunreachable_pub
+$(obj)/core.o: private skip_flags = -Wunreachable_pub \
+ -Wfuzzy_provenance_casts -Wlossy_provenance_casts
$(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
$(obj)/core.o: private rustc_target_flags = $(core-cfgs)
$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 014af0d1fc70..185bf29e44d9 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -9,6 +9,14 @@
//! using this crate.
#![no_std]
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
// See <https://github.com/rust-lang/rust-bindgen/issues/1651>.
#![cfg_attr(test, allow(deref_nullptr))]
#![cfg_attr(test, allow(unaligned_references))]
diff --git a/rust/build_error.rs b/rust/build_error.rs
index fa24eeef9929..84e24598857f 100644
--- a/rust/build_error.rs
+++ b/rust/build_error.rs
@@ -18,6 +18,14 @@
//! [const-context]: https://doc.rust-lang.org/reference/const_eval.html#const-context
#![no_std]
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
/// Panics if executed in [const context][const-context], or triggers a build error if not.
///
diff --git a/rust/compiler_builtins.rs b/rust/compiler_builtins.rs
index f14b8d7caf89..0dcb25a644f6 100644
--- a/rust/compiler_builtins.rs
+++ b/rust/compiler_builtins.rs
@@ -21,6 +21,14 @@
#![allow(internal_features)]
#![feature(compiler_builtins)]
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
#![compiler_builtins]
#![no_builtins]
#![no_std]
diff --git a/rust/ffi.rs b/rust/ffi.rs
index 584f75b49862..28a5e9a09b70 100644
--- a/rust/ffi.rs
+++ b/rust/ffi.rs
@@ -8,6 +8,14 @@
//! C ABI. The kernel does not use [`core::ffi`], so it can customise the mapping that deviates from
//! the platform default.
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
#![no_std]
macro_rules! alias {
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 398242f92a96..6fd4fb2176aa 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -13,6 +13,14 @@
#![no_std]
#![feature(arbitrary_self_types)]
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
#![cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, feature(derive_coerce_pointee))]
#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 8c7b786377ee..91450de998d3 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -2,6 +2,15 @@
//! Crate for all kernel procedural macros.
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
+
// When fixdep scans this, it will find this string `CONFIG_RUSTC_VERSION_TEXT`
// and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is
// touched by Kconfig when the version string from the compiler changes.
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 13495910271f..84ef3828e0d4 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -8,6 +8,14 @@
//! userspace APIs.
#![no_std]
+#![cfg_attr(
+ not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),
+ feature(strict_provenance)
+)]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints)
+)]
// See <https://github.com/rust-lang/rust-bindgen/issues/1651>.
#![cfg_attr(test, allow(deref_nullptr))]
#![cfg_attr(test, allow(unaligned_references))]
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 993708d11874..021ee36ae8f2 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -226,7 +226,10 @@ $(obj)/%.lst: $(obj)/%.c FORCE
# Compile Rust sources (.rs)
# ---------------------------------------------------------------------------
-rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons
+# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
+#
+# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
+rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,$(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
# `--out-dir` is required to avoid temporaries being created by `rustc` in the
# current working directory, which may be not accessible in the out-of-tree
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-13 14:11 ` Benno Lossin
@ 2025-03-13 17:50 ` Tamir Duberstein
2025-03-13 18:12 ` Benno Lossin
0 siblings, 1 reply; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-13 17:50 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Thu, Mar 13, 2025 at 10:11 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Thu Mar 13, 2025 at 11:47 AM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 6:38 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
> >> > On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >>
> >> >> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
> >> >> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >> >> >>
> >> >> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> >> > Always enable the features, we have `allow(stable_features)` for this
> >> >> >> > reason (then you don't have to do this dance with checking if it's
> >> >> >> > already stable or not :)
> >> >> >>
> >> >> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
> >> >> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
> >> >> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
> >> >> >> need to read the config to learn that you need to enable
> >> >> >> `feature(strict_provenance_lints)`.
> >> >>
> >> >> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
> >> >> bit of a bummer...
> >> >>
> >> >> But I guess we could have this config option (in `init/Kconfig`):
> >> >>
> >> >> config RUSTC_HAS_STRICT_PROVENANCE
> >> >> def_bool RUSTC_VERSION >= 108400
> >> >>
> >> >> and then do this in `lib.rs`:
> >> >>
> >> >> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
> >> >
> >> > Yep! That's exactly what I did, but as I mentioned up-thread, the
> >> > result is that we only cover the `kernel` crate.
> >>
> >> Ah I see, can't we just have the above line in the other crate roots?
> >
> > The most difficult case is doctests. You'd have to add this to every
> > example AFAICT.
> >
> >> >> > Actually this isn't even the only problem. It seems that
> >> >> > `-Astable_features` doesn't affect features enabled on the command
> >> >> > line at all:
> >> >> >
> >> >> > error[E0725]: the feature `strict_provenance` is not in the list of
> >> >> > allowed features
> >> >> > --> <crate attribute>:1:9
> >> >> > |
> >> >> > 1 | feature(strict_provenance)
> >> >> > | ^^^^^^^^^^^^^^^^^
> >> >>
> >> >> That's because you need to append the feature to `rust_allowed_features`
> >> >> in `scripts/Makefile.build` (AFAIK).
> >> >
> >> > Thanks, that's a helpful pointer, and it solves some problems but not
> >> > all. The root Makefile contains this bit:
> >> >
> >> >> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> >> >> -Zallow-features= $(HOSTRUSTFLAGS)
> >> >
> >> > which means we can't use the provenance lints against these host
> >> > targets (including e.g. `rustdoc_test_gen`). We can't remove this
> >> > -Zallow-features= either because then core fails to compile.
> >> >
> >> > I'm at the point where I think I need more involved help. Want to take
> >> > a look at my attempt? It's here:
> >> > https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
>
> With doing `allow(clippy::incompatible_msrv)`, I meant doing that
> globally, not having a module to re-export the functions :)
Yeah, but I think that's too big a hammer. It's a useful warning, and
it doesn't accept per-item configuration.
> >> I'll take a look tomorrow, you're testing my knowledge of the build
> >> system a lot here :)
> >
> > We're guaranteed to learn something :)
>
> Yep! I managed to get it working, but it is rather janky and
> experimental. I don't think you should use this in your patch series
> unless Miguel has commented on it.
>
> Notable things in the diff below:
> * the hostrustflags don't get the *provenance_casts lints (which is
> correct, I think, but probably not the way I did it with filter-out)
> * the crates compiler_builtins, bindings, uapi, build_error, libmacros,
> ffi, etc do get them, but probably shouldn't?
Why don't we want host programs to have the same warnings applied? Why
don't we want it for all those other crates?
I'd expect we want uniform diagnostics settings throughout which is
why these things are in the Makefile rather than in individual crates
in the first place.
Your patch sidesteps the problems I'm having by not applying these
lints to host crates, but I think we should.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-13 17:50 ` Tamir Duberstein
@ 2025-03-13 18:12 ` Benno Lossin
2025-03-14 12:26 ` Tamir Duberstein
0 siblings, 1 reply; 38+ messages in thread
From: Benno Lossin @ 2025-03-13 18:12 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Thu Mar 13, 2025 at 6:50 PM CET, Tamir Duberstein wrote:
> On Thu, Mar 13, 2025 at 10:11 AM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Thu Mar 13, 2025 at 11:47 AM CET, Tamir Duberstein wrote:
>> > On Wed, Mar 12, 2025 at 6:38 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >>
>> >> On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
>> >> > On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >>
>> >> >> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
>> >> >> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> >> >> >>
>> >> >> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> >> > Always enable the features, we have `allow(stable_features)` for this
>> >> >> >> > reason (then you don't have to do this dance with checking if it's
>> >> >> >> > already stable or not :)
>> >> >> >>
>> >> >> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
>> >> >> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
>> >> >> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
>> >> >> >> need to read the config to learn that you need to enable
>> >> >> >> `feature(strict_provenance_lints)`.
>> >> >>
>> >> >> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
>> >> >> bit of a bummer...
>> >> >>
>> >> >> But I guess we could have this config option (in `init/Kconfig`):
>> >> >>
>> >> >> config RUSTC_HAS_STRICT_PROVENANCE
>> >> >> def_bool RUSTC_VERSION >= 108400
>> >> >>
>> >> >> and then do this in `lib.rs`:
>> >> >>
>> >> >> #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
>> >> >
>> >> > Yep! That's exactly what I did, but as I mentioned up-thread, the
>> >> > result is that we only cover the `kernel` crate.
>> >>
>> >> Ah I see, can't we just have the above line in the other crate roots?
>> >
>> > The most difficult case is doctests. You'd have to add this to every
>> > example AFAICT.
>> >
>> >> >> > Actually this isn't even the only problem. It seems that
>> >> >> > `-Astable_features` doesn't affect features enabled on the command
>> >> >> > line at all:
>> >> >> >
>> >> >> > error[E0725]: the feature `strict_provenance` is not in the list of
>> >> >> > allowed features
>> >> >> > --> <crate attribute>:1:9
>> >> >> > |
>> >> >> > 1 | feature(strict_provenance)
>> >> >> > | ^^^^^^^^^^^^^^^^^
>> >> >>
>> >> >> That's because you need to append the feature to `rust_allowed_features`
>> >> >> in `scripts/Makefile.build` (AFAIK).
>> >> >
>> >> > Thanks, that's a helpful pointer, and it solves some problems but not
>> >> > all. The root Makefile contains this bit:
>> >> >
>> >> >> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
>> >> >> -Zallow-features= $(HOSTRUSTFLAGS)
>> >> >
>> >> > which means we can't use the provenance lints against these host
>> >> > targets (including e.g. `rustdoc_test_gen`). We can't remove this
>> >> > -Zallow-features= either because then core fails to compile.
>> >> >
>> >> > I'm at the point where I think I need more involved help. Want to take
>> >> > a look at my attempt? It's here:
>> >> > https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
>>
>> With doing `allow(clippy::incompatible_msrv)`, I meant doing that
>> globally, not having a module to re-export the functions :)
>
> Yeah, but I think that's too big a hammer. It's a useful warning, and
> it doesn't accept per-item configuration.
Hmm, I don't think it's as useful. We're going to be using more unstable
features until we eventually bump the minimum version when we can
disable `RUSTC_BOOTSTRAP=1`. From that point onwards, it will be very
useful, but before that I don't think that it matters too much. Maybe
the others disagree.
>> >> I'll take a look tomorrow, you're testing my knowledge of the build
>> >> system a lot here :)
>> >
>> > We're guaranteed to learn something :)
>>
>> Yep! I managed to get it working, but it is rather janky and
>> experimental. I don't think you should use this in your patch series
>> unless Miguel has commented on it.
>>
>> Notable things in the diff below:
>> * the hostrustflags don't get the *provenance_casts lints (which is
>> correct, I think, but probably not the way I did it with filter-out)
>> * the crates compiler_builtins, bindings, uapi, build_error, libmacros,
>> ffi, etc do get them, but probably shouldn't?
>
> Why don't we want host programs to have the same warnings applied? Why
> don't we want it for all those other crates?
I have never looked at the rust hostprogs before, so I'm missing some
context here.
I didn't enable them, because they are currently being compiled without
any unstable features and I thought we might want to keep that. (though
I don't really see a reason not to compile them with unstable features
that we also use for the kernel crate)
> I'd expect we want uniform diagnostics settings throughout which is
> why these things are in the Makefile rather than in individual crates
> in the first place.
>
> Your patch sidesteps the problems I'm having by not applying these
> lints to host crates, but I think we should.
We're probably working on some stuff that Miguel's new build system will
do entirely differently. So I wouldn't worry too much about getting it
perfect, as it will be removed in a couple cycles.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint
2025-03-13 18:12 ` Benno Lossin
@ 2025-03-14 12:26 ` Tamir Duberstein
0 siblings, 0 replies; 38+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:26 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Thu, Mar 13, 2025 at 2:12 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Thu Mar 13, 2025 at 6:50 PM CET, Tamir Duberstein wrote:
> > On Thu, Mar 13, 2025 at 10:11 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >>
> >> With doing `allow(clippy::incompatible_msrv)`, I meant doing that
> >> globally, not having a module to re-export the functions :)
> >
> > Yeah, but I think that's too big a hammer. It's a useful warning, and
> > it doesn't accept per-item configuration.
>
> Hmm, I don't think it's as useful. We're going to be using more unstable
> features until we eventually bump the minimum version when we can
> disable `RUSTC_BOOTSTRAP=1`. From that point onwards, it will be very
> useful, but before that I don't think that it matters too much. Maybe
> the others disagree.
I'd rather keep this narrowly scoped for now -- putting the genie back
in the bottle later is usually harder.
> > Why don't we want host programs to have the same warnings applied? Why
> > don't we want it for all those other crates?
>
> I have never looked at the rust hostprogs before, so I'm missing some
> context here.
>
> I didn't enable them, because they are currently being compiled without
> any unstable features and I thought we might want to keep that. (though
> I don't really see a reason not to compile them with unstable features
> that we also use for the kernel crate)
>
> > I'd expect we want uniform diagnostics settings throughout which is
> > why these things are in the Makefile rather than in individual crates
> > in the first place.
> >
> > Your patch sidesteps the problems I'm having by not applying these
> > lints to host crates, but I think we should.
>
> We're probably working on some stuff that Miguel's new build system will
> do entirely differently. So I wouldn't worry too much about getting it
> perfect, as it will be removed in a couple cycles.
I got it working, but it's pretty messy. Let's discuss on v3.
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2025-03-14 12:27 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-09 16:00 [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 1/5] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
2025-03-12 14:22 ` Benno Lossin
2025-03-12 15:13 ` Tamir Duberstein
2025-03-09 16:00 ` [PATCH v2 2/5] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
2025-03-12 14:39 ` Benno Lossin
2025-03-12 15:18 ` Tamir Duberstein
2025-03-12 15:25 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 3/5] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
2025-03-12 14:40 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 4/5] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
2025-03-11 13:08 ` Benno Lossin
2025-03-09 16:00 ` [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint Tamir Duberstein
2025-03-12 15:05 ` Benno Lossin
2025-03-12 15:35 ` Tamir Duberstein
2025-03-12 15:49 ` Miguel Ojeda
2025-03-12 17:05 ` Benno Lossin
2025-03-12 18:01 ` Tamir Duberstein
2025-03-12 19:10 ` Benno Lossin
2025-03-12 19:19 ` Tamir Duberstein
2025-03-12 19:43 ` Benno Lossin
2025-03-12 20:07 ` Tamir Duberstein
2025-03-12 20:41 ` Tamir Duberstein
2025-03-12 21:01 ` Benno Lossin
2025-03-12 21:04 ` Tamir Duberstein
2025-03-12 21:10 ` Tamir Duberstein
2025-03-12 21:30 ` Benno Lossin
2025-03-12 22:24 ` Tamir Duberstein
2025-03-12 22:38 ` Benno Lossin
2025-03-13 10:47 ` Tamir Duberstein
2025-03-13 14:11 ` Benno Lossin
2025-03-13 17:50 ` Tamir Duberstein
2025-03-13 18:12 ` Benno Lossin
2025-03-14 12:26 ` Tamir Duberstein
2025-03-12 20:42 ` Benno Lossin
2025-03-12 15:41 ` Miguel Ojeda
2025-03-12 15:07 ` [PATCH v2 0/5] rust: reduce pointer casts, enable related lints Benno Lossin
2025-03-12 15:14 ` Tamir Duberstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).