rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: kernel: add missing safety comments
@ 2025-08-03 17:10 Jinheng LI
  2025-08-03 17:44 ` Miguel Ojeda
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jinheng LI @ 2025-08-03 17:10 UTC (permalink / raw)
  To: ojeda, rust-for-linux
  Cc: linux-kernel, alex.gaynor, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross

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)

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

* Re: [PATCH] rust: kernel: add missing safety comments
  2025-08-03 17:10 [PATCH] rust: kernel: add missing safety comments Jinheng LI
@ 2025-08-03 17:44 ` Miguel Ojeda
  2025-08-03 17:47 ` Miguel Ojeda
  2025-08-03 19:37 ` Benno Lossin
  2 siblings, 0 replies; 4+ messages in thread
From: Miguel Ojeda @ 2025-08-03 17:44 UTC (permalink / raw)
  To: Jinheng LI
  Cc: ojeda, rust-for-linux, linux-kernel, alex.gaynor, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross

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

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

* Re: [PATCH] rust: kernel: add missing safety comments
  2025-08-03 17:10 [PATCH] rust: kernel: add missing safety comments Jinheng LI
  2025-08-03 17:44 ` Miguel Ojeda
@ 2025-08-03 17:47 ` Miguel Ojeda
  2025-08-03 19:37 ` Benno Lossin
  2 siblings, 0 replies; 4+ messages in thread
From: Miguel Ojeda @ 2025-08-03 17:47 UTC (permalink / raw)
  To: Jinheng LI
  Cc: ojeda, rust-for-linux, linux-kernel, alex.gaynor, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross

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

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

* Re: [PATCH] rust: kernel: add missing safety comments
  2025-08-03 17:10 [PATCH] rust: kernel: add missing safety comments Jinheng LI
  2025-08-03 17:44 ` Miguel Ojeda
  2025-08-03 17:47 ` Miguel Ojeda
@ 2025-08-03 19:37 ` Benno Lossin
  2 siblings, 0 replies; 4+ messages in thread
From: Benno Lossin @ 2025-08-03 19:37 UTC (permalink / raw)
  To: Jinheng LI, ojeda, rust-for-linux
  Cc: linux-kernel, alex.gaynor, gary, bjorn3_gh, a.hindborg, aliceryhl,
	tmgross

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

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

end of thread, other threads:[~2025-08-03 19:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-03 17:10 [PATCH] rust: kernel: add missing safety comments Jinheng LI
2025-08-03 17:44 ` Miguel Ojeda
2025-08-03 17:47 ` Miguel Ojeda
2025-08-03 19:37 ` Benno Lossin

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).