[PATCH] rust: num: bounded: add safety comment for Bounded::__new

[PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Hsiu Che Yu]

The `__new` constructor only performs a bounds check and stores the
value in the wrapper type. It does not perform any unsafe memory
operations, so it does not need to be marked as `unsafe`.

Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1211

Signed-off-by: Hsiu Che Yu <yu.whisper.personal@gmail.com>
---
 rust/kernel/num/bounded.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
index f870080af8ac..cde1b8ba6880 100644
--- a/rust/kernel/num/bounded.rs
+++ b/rust/kernel/num/bounded.rs
@@ -284,6 +284,9 @@ impl<T, const N: u32> Bounded<T, N>
     ///
     /// The caller remains responsible for checking, either statically or dynamically, that `value`
     /// can be represented as a `T` using at most `N` bits.
+    ///
+    /// **Note**: This function is not marked as `unsafe` because it performs no memory-unsafe
+    /// operations itself.
     const fn __new(value: T) -> Self {
         // Enforce the type invariants.
         const {
-- 
2.43.0

Re: [PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Alice Ryhl]

On Mon, Dec 01, 2025 at 02:25:16PM +0800, Hsiu Che Yu wrote:
> The `__new` constructor only performs a bounds check and stores the
> value in the wrapper type. It does not perform any unsafe memory
> operations, so it does not need to be marked as `unsafe`.
> 
> Reported-by: Miguel Ojeda <ojeda@kernel.org>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1211
> 
> Signed-off-by: Hsiu Che Yu <yu.whisper.personal@gmail.com>
> ---
>  rust/kernel/num/bounded.rs | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
> index f870080af8ac..cde1b8ba6880 100644
> --- a/rust/kernel/num/bounded.rs
> +++ b/rust/kernel/num/bounded.rs
> @@ -284,6 +284,9 @@ impl<T, const N: u32> Bounded<T, N>
>      ///
>      /// The caller remains responsible for checking, either statically or dynamically, that `value`
>      /// can be represented as a `T` using at most `N` bits.
> +    ///
> +    /// **Note**: This function is not marked as `unsafe` because it performs no memory-unsafe
> +    /// operations itself.

I disagree. For the same reasons as str::from_utf8_unchecked, this
should also be unsafe. It creates a value that violates invariants,
which may be used to trigger UB combined with other safe code.

Alice

Re: [PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Hsiu Che Yu]

On Mon, Dec 01, 2025 at 10:12:27AM +0000, Alice Ryhl wrote:
>I disagree. For the same reasons as str::from_utf8_unchecked, this
>should also be unsafe. It creates a value that violates invariants,
>which may be used to trigger UB combined with other safe code.
>
>Alice

I understand. Once the caller passes a value that cannot be represented 
with N bits, the type invariant is violated. Keeping this function safe 
could allow callers to overlook this critical requirement.

Thank you for the feedback. I will submit v2 with the necessary changes 
and improved documentation.

Best regards,
Hsiu Che Yu

Re: [PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 7:26 AM Hsiu Che Yu
<yu.whisper.personal@gmail.com> wrote:
>
> Reported-by: Miguel Ojeda <ojeda@kernel.org>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1211

So typically we have "fixes" or "improvements". The former ones
typically have Reported-by and Closes (and others like Fixes), while
improvements don't (and instead Suggested-by would be used in this
case).

I created the issue in this way to have you think about whether it
should be `unsafe fn` or not, and depending on the solution, the
eventual patch would be considered a fix (i.e. making it `unsafe fn`,
since it would not be intentional) or an improvement (i.e. documenting
why it is not unsafe, since it would have been intentionally safe).

Here you considered the solution to be that it should not be unsafe,
in which case it wouldn't be a fix and thus those tags wouldn't be
used.

The solution to the puzzle is now revealed, and indeed it should be
`unsafe fn` (even if it is private), so it is indeed a fix (but not
this fix, of course :).

[ In particular, functions having unsafe code inside of them is
orthogonal to them being unsafe functions or not, e.g. you may have
also safe functions with `unsafe` blocks inside. ]

For v2, you should consider what documentation you should add to make
it `unsafe fn` (please build with `CLIPPY=1` to check) and what others
changes would be needed.

Thanks for the patch!

Cheers,
Miguel

Re: [PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Hsiu Che Yu]

On Mon, Dec 01, 2025 at 01:44:24PM +0100, Miguel Ojeda wrote:
>So typically we have "fixes" or "improvements". The former ones
>typically have Reported-by and Closes (and others like Fixes), while
>improvements don't (and instead Suggested-by would be used in this
>case).
>
>I created the issue in this way to have you think about whether it
>should be `unsafe fn` or not, and depending on the solution, the
>eventual patch would be considered a fix (i.e. making it `unsafe fn`,
>since it would not be intentional) or an improvement (i.e. documenting
>why it is not unsafe, since it would have been intentionally safe).
>
>Here you considered the solution to be that it should not be unsafe,
>in which case it wouldn't be a fix and thus those tags wouldn't be
>used.
>
>The solution to the puzzle is now revealed, and indeed it should be
>`unsafe fn` (even if it is private), so it is indeed a fix (but not
>this fix, of course :).
>
>[ In particular, functions having unsafe code inside of them is
>orthogonal to them being unsafe functions or not, e.g. you may have
>also safe functions with `unsafe` blocks inside. ]
>
>For v2, you should consider what documentation you should add to make
>it `unsafe fn` (please build with `CLIPPY=1` to check) and what others
>changes would be needed.
>
>Thanks for the patch!
>
>Cheers,
>Miguel

I previously believed that a function should only be marked unsafe when 
it directly operates on unsafe code. I now understand that the decision 
should be based on the actual safety implications rather than just 
semantic considerations.

Thank you also for the clarification on the tags. I spent some time 
trying to understand them, and your explanation is very helpful.

I will address this in v2 by making it an `unsafe fn` and documenting
the safety requirements in the `# Safety` section.

Best regards,
Hsiu Che Yu

Re: [PATCH] rust: num: bounded: add safety comment for Bounded::__new

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 2:35 PM Hsiu Che Yu
<yu.whisper.personal@gmail.com> wrote:
>
> I previously believed that a function should only be marked unsafe when
> it directly operates on unsafe code. I now understand that the decision
> should be based on the actual safety implications rather than just
> semantic considerations.
>
> Thank you also for the clarification on the tags. I spent some time
> trying to understand them, and your explanation is very helpful.
>
> I will address this in v2 by making it an `unsafe fn` and documenting
> the safety requirements in the `# Safety` section.

My pleasure, and welcome to the Linux kernel!

Cheers,
Miguel