The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [[PATCH]] rust: kernel: add SAFETY comments in aref doc example
@ 2026-07-12 12:35 Christopher Joshy
  2026-07-12 12:42 ` Miguel Ojeda
  2026-07-12 15:30 ` [PATCH v2] " christopherjoshy
  0 siblings, 2 replies; 4+ messages in thread
From: Christopher Joshy @ 2026-07-12 12:35 UTC (permalink / raw)
  To: ojeda; +Cc: rust-for-linux, linux-kernel, Christopher Joshy

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


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

* Re: [[PATCH]] rust: kernel: add SAFETY comments in aref doc example
  2026-07-12 12:35 [[PATCH]] rust: kernel: add SAFETY comments in aref doc example Christopher Joshy
@ 2026-07-12 12:42 ` Miguel Ojeda
  2026-07-12 15:30 ` [PATCH v2] " christopherjoshy
  1 sibling, 0 replies; 4+ messages in thread
From: Miguel Ojeda @ 2026-07-12 12:42 UTC (permalink / raw)
  To: Christopher Joshy; +Cc: ojeda, rust-for-linux, linux-kernel

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

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

* [PATCH v2] rust: kernel: add SAFETY comments in aref doc example
  2026-07-12 12:35 [[PATCH]] rust: kernel: add SAFETY comments in aref doc example Christopher Joshy
  2026-07-12 12:42 ` Miguel Ojeda
@ 2026-07-12 15:30 ` christopherjoshy
  2026-07-12 15:49   ` [PATCH v3] " christopherjoshy
  1 sibling, 1 reply; 4+ messages in thread
From: christopherjoshy @ 2026-07-12 15:30 UTC (permalink / raw)
  To: ojeda
  Cc: boqun, gary, aliceryhl, lyude, daniel.almeida, work, bjorn3_gh,
	lossin, a.hindborg, tmgross, dakr, tamird, acourbot, linux-kernel,
	rust-for-linux, Christopher Joshy

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


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

* [PATCH v3] rust: kernel: add SAFETY comments in aref doc example
  2026-07-12 15:30 ` [PATCH v2] " christopherjoshy
@ 2026-07-12 15:49   ` christopherjoshy
  0 siblings, 0 replies; 4+ messages in thread
From: christopherjoshy @ 2026-07-12 15:49 UTC (permalink / raw)
  To: ojeda
  Cc: boqun, gary, aliceryhl, lyude, daniel.almeida, work, bjorn3_gh,
	lossin, a.hindborg, tmgross, dakr, tamird, acourbot, linux-kernel,
	rust-for-linux, Christopher Joshy

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


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

end of thread, other threads:[~2026-07-12 15:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12 12:35 [[PATCH]] rust: kernel: add SAFETY comments in aref doc example Christopher Joshy
2026-07-12 12:42 ` Miguel Ojeda
2026-07-12 15:30 ` [PATCH v2] " christopherjoshy
2026-07-12 15:49   ` [PATCH v3] " christopherjoshy

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