From 5cba005b59a032fc80f818b393b7e4c36a460710 Mon Sep 17 00:00:00 2001
From: Jinheng Li <ahengljh@gmail.com>
Date: Mon, 4 Aug 2025 00:56:11 +0800
Subject: [PATCH] rust: kernel: add missing safety comments

Add safety documentation for unsafe functions that were missing proper
SAFETY comments. This improves code maintainability and helps
developers understand the safety requirements.

- str.rs: Document safety requirements for as_str_unchecked()
- list.rs: Document safety requirements for remove() method

These functions had TODO markers for safety documentation that are
now properly filled in with clear explanations of the invariants
and caller responsibilities.

Signed-off-by: Jinheng Li <ahengljh@gmail.com>
---
rust/kernel/list.rs | 5 ++++-
rust/kernel/str.rs  | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index c391c30b80f8..b9dbb73a7ebe 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -456,7 +456,10 @@ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
  ///
  /// `item` must not be in a different linked list (with the same id).
  pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
-        // SAFETY: TODO.
+        // SAFETY: The caller guarantees that `item` is not in a
different linked list with the
+        // same ID. Since we have a mutable reference to the list, we
have exclusive access to all
+        // items in this list. The `view_links` and `fields`
functions are safe to call on any
+        // item reference, and will return the location of the list
links for this item.
      let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
      // SAFETY: The user provided a reference, and reference are
never dangling.
      //
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index a927db8e079c..8fe9a15fc16e 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -349,7 +349,10 @@ pub fn to_str(&self) -> Result<&str,
core::str::Utf8Error> {
  /// ```
  #[inline]
  pub unsafe fn as_str_unchecked(&self) -> &str {
-        // SAFETY: TODO.
+        // SAFETY: The caller guarantees that this `CStr` contains
only valid UTF-8 bytes.
+        // Since `CStr` is guaranteed to contain no interior null
bytes (by its invariants),
+        // and we're excluding the trailing null byte via
`as_bytes()`, the resulting slice
+        // is valid for `from_utf8_unchecked`.
      unsafe { core::str::from_utf8_unchecked(self.as_bytes()) }
  }

-- 
2.39.5 (Apple Git-154)

On Sun, Aug 3, 2025 at 7:10 PM Jinheng LI <ahengljh@gmail.com> wrote:
>
> From 5cba005b59a032fc80f818b393b7e4c36a460710 Mon Sep 17 00:00:00 2001
> From: Jinheng Li <ahengljh@gmail.com>
> Date: Mon, 4 Aug 2025 00:56:11 +0800
> Subject: [PATCH] rust: kernel: add missing safety comments

This is supposed to be used by e.g. Git `send-email` to craft the
email, rather than being sent in the body.

> Add safety documentation for unsafe functions that were missing proper
> SAFETY comments. This improves code maintainability and helps
> developers understand the safety requirements.
>
> - str.rs: Document safety requirements for as_str_unchecked()
> - list.rs: Document safety requirements for remove() method
>
> These functions had TODO markers for safety documentation that are
> now properly filled in with clear explanations of the invariants
> and caller responsibilities.

These paragraphs all sound as if we are documenting the safety
requirements of the outer function. However, `// SAFETY: ...` comments
are meant to explain how we satisfy the safety requirements of the
functions etc. used within the `unsafe` block that follows.

Also, to avoid confusion, we normally use the word "documentation" for
the `///` ones, i.e. things that are meant for users of APIs.

So I think the commit message is a bit confusing.

The contents are also a bit strange, e.g.:

> +        // Since `CStr` is guaranteed to contain no interior null
> bytes (by its invariants),
> +        // and we're excluding the trailing null byte via
> `as_bytes()`, the resulting slice
> +        // is valid for `from_utf8_unchecked`.

`from_utf8_unchecked`'s requirements are just that "The bytes passed
in must be valid UTF-8.". Why does the NUL matter here?

Thanks!

Cheers,
Miguel

On Sun, Aug 3, 2025 at 7:10 PM Jinheng LI <ahengljh@gmail.com> wrote:
>
> Signed-off-by: Jinheng Li <ahengljh@gmail.com>

By the way, one more tag:

Link: https://github.com/Rust-for-Linux/linux/issues/351

Cheers,
Miguel

On Sun Aug 3, 2025 at 7:10 PM CEST, Jinheng LI wrote:
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index c391c30b80f8..b9dbb73a7ebe 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -456,7 +456,10 @@ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
>   ///
>   /// `item` must not be in a different linked list (with the same id).
>   pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
> -        // SAFETY: TODO.
> +        // SAFETY: The caller guarantees that `item` is not in a
> different linked list with the

This looks like your email client wrapped the diff, please take a look
at [1] and perform the necessary steps for your email client. Otherwise
it won't be easy to apply your patch.

[1]: https://docs.kernel.org/process/email-clients.html

---
Cheers,
Benno

> +        // same ID. Since we have a mutable reference to the list, we
> have exclusive access to all