* [[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