] rust: kernel: add SAFETY comments in aref doc example
PATCH 1/1 ] rust: kernel: add SAFETY comments in aref doc example
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.
--- rust/kernel/sync/aref.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
rust/kernel/sync/aref.rs +4 -2
@@ -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); ///
Signature · 1 line
2.43.0
Quoted 4 lines · Christopher Joshy
Replace TODO safety comments with proper explanations in the
ARef::into_rawdoc example.
Quoted 8 lines
For the
unsafe impl AlwaysRefCounted, explain that the empty implementation is trivially sound sinceEmptyhas no data.For the
ARef::from_rawcall, explain that the pointer is valid because it is derived from a valid reference.
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 1/1 rust: kernel: add SAFETY comments in aref doc example
In reply to Christopher JoshyFrom: Christopher Joshy <[email protected]>
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.
--- rust/kernel/sync/aref.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
rust/kernel/sync/aref.rs +9 -2
@@ -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); ///
Signature · 1 line
2.43.0
PATCH v3 1/1 rust: kernel: add SAFETY comments in aref doc example
From: Christopher Joshy <[email protected]>
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.
--- rust/kernel/sync/aref.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
rust/kernel/sync/aref.rs +8 -2
@@ -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); ///
Signature · 1 line
2.43.0