Re: [PATCH v20 6/8] rust: Add missing SAFETY documentation for `ARef` example

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Mismatched and incorrect SAFETY comments in the `ARef::into_raw` doctest, alongside an unsound `RefCounted` trait implementation.
--

--- Patch [6]: [PATCH v20 6/8] rust: Add missing SAFETY documentation for `ARef` example ---
commit 1891eeeba9b2ed0af7d99f9518dcd85941d2518c
Author: Oliver Mangold <oliver.mangold@pm.me>

rust: Add missing SAFETY documentation for ARef example

This commit updates the rustdoc example for ARef to replace the "TODO"
SAFETY comments with explanations for the RefCounted trait implementation
and ARef::from_raw call.

> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
> @@ -142,7 +142,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// struct Empty {}
>      ///
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and never
> +    /// // frees the underlying object. Thus we can act as owning an increment on the refcount for
> +    /// // the object that we pass to the newly created `ARef`.
>      /// unsafe impl RefCounted for Empty {
>      ///     fn inc_ref(&self) {}
>      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}

[Severity: Low]
Are the SAFETY comments for the trait implementation and from_raw call swapped
here?

This comment justifies the caller requirement of ARef::from_raw (owning an
increment), rather than the trait's own safety requirement (that increments keep
the object alive).

Additionally, does this explanation overlook the fact that the stack-allocated
variable data will eventually go out of scope and be freed?

If a developer copies this pattern and calls ARef::clone(), could it extend the
pointer's lifetime beyond the stack frame without keeping the underlying object
alive?

> @@ -150,7 +152,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// let mut data = Empty {};
>      /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: We keep `data` around longer than the `ARef`.
>      /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };

[Severity: Low]
Does this comment justify the object's lifetime (which is the responsibility of
the RefCounted trait implementation) instead of addressing the specific safety
requirement of ARef::from_raw?

The from_raw method requires the caller to own a reference count increment.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-unique-ref-v20-0-490735672187@kernel.org?part=6