Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH] rust: cpu: use `unsafe_precondition_assert!`
@ 2026-05-18 17:44 Ritvik Gupta
  2026-05-18 17:51 ` Miguel Ojeda
  0 siblings, 1 reply; 3+ messages in thread
From: Ritvik Gupta @ 2026-05-18 17:44 UTC (permalink / raw)
  To: tglx, peterz, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, dakr
  Cc: linux-kernel, rust-for-linux, skhan

Replace `debug_assert!` invocations with
`unsafe_precondition_assert!` in `from_i32_unchecked`
and `from_u32_unchecked` unsafe functions.

Signed-off-by: Ritvik Gupta <ritvikfoss@gmail.com>
---
 rust/kernel/cpu.rs | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index cb6c0338ef5a..e33a87304c59 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -4,7 +4,7 @@
 //!
 //! C header: [`include/linux/cpu.h`](srctree/include/linux/cpu.h)
 
-use crate::{bindings, device::Device, error::Result, prelude::ENODEV};
+use crate::{bindings, device::Device, error::Result, prelude::ENODEV, unsafe_precondition_assert};
 
 /// Returns the maximum number of possible CPUs in the current system configuration.
 #[inline]
@@ -54,8 +54,14 @@ impl CpuId {
     /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
     #[inline]
     pub unsafe fn from_i32_unchecked(id: i32) -> Self {
-        debug_assert!(id >= 0);
-        debug_assert!((id as u32) < nr_cpu_ids());
+        unsafe_precondition_assert!(
+            id >= 0,
+            "CpuId::from_i32_unchecked requires a non-negative 'id'"
+        );
+        unsafe_precondition_assert!(
+            (id as u32) < nr_cpu_ids(),
+            "CpuId::from_i32_unchecked requires a valid 'id' (i.e., `0 <= id < nr_cpu_ids()`)"
+        );
 
         // INVARIANT: The function safety guarantees `id` is a valid CPU id.
         Self(id as u32)
@@ -78,10 +84,16 @@ pub fn from_i32(id: i32) -> Option<Self> {
     /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
     #[inline]
     pub unsafe fn from_u32_unchecked(id: u32) -> Self {
-        debug_assert!(id < nr_cpu_ids());
+        unsafe_precondition_assert!(
+            id < nr_cpu_ids(),
+            "CpuId::from_i32_unchecked requires a valid 'id' (i.e., `0 <= id < nr_cpu_ids()`)"
+        );
 
         // Ensure the `id` fits in an [`i32`] as it's also representable that way.
-        debug_assert!(id <= i32::MAX as u32);
+        unsafe_precondition_assert!(
+            id <= i32::MAX as u32,
+            "CpuId::from_u32_unchecked requires the 'id' to not overflow i32::MAX"
+        );
 
         // INVARIANT: The function safety guarantees `id` is a valid CPU id.
         Self(id)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: cpu: use `unsafe_precondition_assert!`
  2026-05-18 17:44 [PATCH] rust: cpu: use `unsafe_precondition_assert!` Ritvik Gupta
@ 2026-05-18 17:51 ` Miguel Ojeda
  2026-05-18 18:53   ` Ritvik Gupta
  0 siblings, 1 reply; 3+ messages in thread
From: Miguel Ojeda @ 2026-05-18 17:51 UTC (permalink / raw)
  To: Ritvik Gupta
  Cc: tglx, peterz, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, linux-kernel, rust-for-linux, skhan

On Mon, May 18, 2026 at 7:40 PM Ritvik Gupta <ritvikfoss@gmail.com> wrote:
>
> Replace `debug_assert!` invocations with
> `unsafe_precondition_assert!` in `from_i32_unchecked`
> and `from_u32_unchecked` unsafe functions.

Thanks for the patch!

This should mention that we are adding messages too. That could
actually be a second patch in the series to make it even clearer, but
it is not a big a deal either way (and perhaps `rustfmt` makes that
first patch not that clean due to the line wrapping anyway).

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1232

> -use crate::{bindings, device::Device, error::Result, prelude::ENODEV};
> +use crate::{bindings, device::Device, error::Result, prelude::ENODEV, unsafe_precondition_assert};

We should probably put it in the prelude along the others -- the
easier it is to type, the better, to encourage its use.

Do you want to add a first patch in the series doing that?

> +            "CpuId::from_i32_unchecked requires a valid 'id' (i.e., `0 <= id < nr_cpu_ids()`)"

This seems to mix Markdown backquotes with single quotes, and then
doesn't use backquotes in the first part. Could we make it consistent
if we are adding the messages? e.g.

    "`CpuId::from_i32_unchecked` requires a valid `id` (i.e., `0 <= id
< nr_cpu_ids()`)"

Sashiko has also spotted a copy-paste error in the one below.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: cpu: use `unsafe_precondition_assert!`
  2026-05-18 17:51 ` Miguel Ojeda
@ 2026-05-18 18:53   ` Ritvik Gupta
  0 siblings, 0 replies; 3+ messages in thread
From: Ritvik Gupta @ 2026-05-18 18:53 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: tglx, peterz, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, linux-kernel, rust-for-linux, skhan

On Mon, May 18, 2026 at 07:51:28PM +0200, Miguel Ojeda wrote:
> On Mon, May 18, 2026 at 7:40 PM Ritvik Gupta <ritvikfoss@gmail.com> wrote:
> >
> > Replace `debug_assert!` invocations with
> > `unsafe_precondition_assert!` in `from_i32_unchecked`
> > and `from_u32_unchecked` unsafe functions.
> 
> Thanks for the patch!
> 
> This should mention that we are adding messages too. That could
> actually be a second patch in the series to make it even clearer, but
> it is not a big a deal either way (and perhaps `rustfmt` makes that
> first patch not that clean due to the line wrapping anyway).
> 
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1232
> 
> > -use crate::{bindings, device::Device, error::Result, prelude::ENODEV};
> > +use crate::{bindings, device::Device, error::Result, prelude::ENODEV, unsafe_precondition_assert};
> 
> We should probably put it in the prelude along the others -- the
> easier it is to type, the better, to encourage its use.
> 
> Do you want to add a first patch in the series doing that?
> 

Sure! In v2 series, I'll add the first patch that adds `unsafe_precondition_assert!` to the prelude and
second patch that uses it.

> > +            "CpuId::from_i32_unchecked requires a valid 'id' (i.e., `0 <= id < nr_cpu_ids()`)"
> 
> This seems to mix Markdown backquotes with single quotes, and then
> doesn't use backquotes in the first part. Could we make it consistent
> if we are adding the messages? e.g.
> 
>     "`CpuId::from_i32_unchecked` requires a valid `id` (i.e., `0 <= id
> < nr_cpu_ids()`)"
> 

Sure, will fix :)

> Sashiko has also spotted a copy-paste error in the one below.
> 
> Cheers,
> Miguel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-18 18:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 17:44 [PATCH] rust: cpu: use `unsafe_precondition_assert!` Ritvik Gupta
2026-05-18 17:51 ` Miguel Ojeda
2026-05-18 18:53   ` Ritvik Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox