] rust: kernel: add SAFETY comments in aref doc example

[[PATCH]] rust: kernel: add SAFETY comments in aref doc example

Posted by Christopher Joshy 1 week, 6 days ago

Replace TODO safety comments with proper explanations in the
`ARef::into_raw` doc example.

For the `unsafe impl AlwaysRefCounted`, explain that the empty
implementation is trivially sound since `Empty` has no data.

For the `ARef::from_raw` call, explain that the pointer is valid
because it is derived from a valid reference.

Link: https://github.com/Rust-for-Linux/linux/issues/351
Signed-off-by: Christopher Joshy <christopherjoshy4@gmail.com>
---
 rust/kernel/sync/aref.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index b721b2e00..932343799 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -124,7 +124,8 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
-    /// # // SAFETY: TODO.
+    /// # // SAFETY: `Empty` has no data, so `inc_ref` and `dec_ref` being no-ops is trivially
+    /// # // sound.
     /// unsafe impl AlwaysRefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -132,7 +133,8 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// let mut data = Empty {};
     /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
-    /// # // SAFETY: TODO.
+    /// # // SAFETY: `ptr` is derived from a valid reference and is therefore non-null, properly
+    /// # // aligned, and points to a valid `Empty` instance.
     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
     ///
-- 
2.43.0

Re: [[PATCH]] rust: kernel: add SAFETY comments in aref doc example

Posted by Miguel Ojeda 1 week, 6 days ago

On Sun, Jul 12, 2026 at 2:35 PM Christopher Joshy
<christopherjoshy4@gmail.com> wrote:
>
> Replace TODO safety comments with proper explanations in the
> `ARef::into_raw` doc example.
>
> For the `unsafe impl AlwaysRefCounted`, explain that the empty
> implementation is trivially sound since `Empty` has no data.
>
> For the `ARef::from_raw` call, explain that the pointer is valid
> because it is derived from a valid reference.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/351
> Signed-off-by: Christopher Joshy <christopherjoshy4@gmail.com>

Thanks for the patch! A few procedural notes:

  - The title seems to have double brackets.

  - Please check the prefix for the title that previous patches to
this file use in the Git log.

  - Now that we have a proper `// SAFETY` comment, consider showing it
in the rendered output instead of hiding it.

  - Please Cc all the relevant maintainers and reviewers, i.e. you can
check `MAINTAINERS` and/or use the `scripts/get_maintainer.pl` (you
will need `RUST [SYNC]` and `RUST`).

  - I personally don't mind either way, but I ask to add Suggested-by
in the "good first issue"s because I want to encourage other reviewers
to open new ones.

Cheers,
Miguel

[PATCH v2] rust: kernel: add SAFETY comments in aref doc example

Posted by christopherjoshy 1 week, 6 days ago

From: Christopher Joshy <christopherjoshy4@gmail.com>

Add the missing `// SAFETY` comments in the `ARef::into_raw` doc
example, replacing the `TODO` placeholders from issue #351.

For the `unsafe impl AlwaysRefCounted`, explain that `Empty` has no
data and its `inc_ref`/`dec_ref` are no-ops, so the liveness
contract is trivially satisfied.

For the `ARef::from_raw` call, explain that `ptr` is derived from a
valid reference (so it is valid) and that the required refcount
increment is trivially owned because `Empty`'s `AlwaysRefCounted`
manages no actual refcount.

Link: https://github.com/Rust-for-Linux/linux/issues/351
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Christopher Joshy <christopherjoshy4@gmail.com>
---
 rust/kernel/sync/aref.rs | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index b721b2e00..5e8b8476b 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -124,7 +124,11 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
-    /// # // SAFETY: TODO.
+    /// // SAFETY: `Empty` has no data and its `inc_ref`/`dec_ref` are no-ops, so the
+    /// // `AlwaysRefCounted` contract holds: `inc_ref` need not keep anything alive (the
+    /// // zero-sized object lives on the stack while `data` is in scope) and `dec_ref` frees
+    /// // nothing. The caller ensures `data` outlives any `ARef<Empty>` (here `data` is a
+    /// // local that outlives `data_ref`).
     /// unsafe impl AlwaysRefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -132,7 +136,10 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// let mut data = Empty {};
     /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
-    /// # // SAFETY: TODO.
+    /// // SAFETY: `ptr` comes from a valid `&mut data` reference, hence it is non-null,
+    /// // aligned and points to a valid `Empty`. `ARef::from_raw` also requires owning a
+    /// // refcount increment; this is trivially met because `Empty`'s `AlwaysRefCounted` uses
+    /// // no-op `inc_ref`/`dec_ref` and so manages no actual refcount.
     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
     ///
-- 
2.43.0

[PATCH v3] rust: kernel: add SAFETY comments in aref doc example

Posted by christopherjoshy 1 week, 6 days ago

From: Christopher Joshy <christopherjoshy4@gmail.com>

Add the missing `// SAFETY` comments in the `ARef::into_raw` doc
example, replacing the `TODO` placeholders from issue #351.

For the `unsafe impl AlwaysRefCounted`, explain that `Empty` owns no
data and has no destructor, so the no-op `inc_ref`/`dec_ref` cannot
free or leak anything, and that this doc example keeps the backing
object alive for the `ARef`'s lifetime.

For the `ARef::from_raw` call, explain that `ptr` is derived from a
valid reference (so it is valid) and that the required refcount
increment is trivially owned because `Empty`'s `AlwaysRefCounted`
manages no actual refcount.

Link: https://github.com/Rust-for-Linux/linux/issues/351
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Christopher Joshy <christopherjoshy4@gmail.com>
---
 rust/kernel/sync/aref.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index b721b2e00..3cf7bc27f 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -124,7 +124,10 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
-    /// # // SAFETY: TODO.
+    /// // SAFETY: `Empty` owns no data and has no destructor, so its `inc_ref`/`dec_ref`
+    /// // are no-ops that cannot free or leak anything. This doc example keeps the backing
+    /// // `Empty` (`data`) alive for the whole lifetime of `data_ref`, so the `ARef` never
+    /// // dereferences freed memory.
     /// unsafe impl AlwaysRefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -132,7 +135,10 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// let mut data = Empty {};
     /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
-    /// # // SAFETY: TODO.
+    /// // SAFETY: `ptr` comes from a valid `&mut data` reference, hence it is non-null,
+    /// // aligned and points to a valid `Empty`. `ARef::from_raw` also requires owning a
+    /// // refcount increment; this is trivially met because `Empty`'s `AlwaysRefCounted` uses
+    /// // no-op `inc_ref`/`dec_ref` and so manages no actual refcount.
     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
     ///
-- 
2.43.0