Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The newly added SAFETY comments in the rustdoc example incorrectly justify an unsound trait implementation that violates the safety invariants of `RefCounted`. --
commit 8c6c2bf519778f423165d5e3c916d3965bebb093 Author: Oliver Mangold <[email protected]> rust: Add missing SAFETY documentation for ARef example This commit replaces TODO safety comments in the rustdoc example for ARef::into_raw with explanations of why the unsafe operations are permitted. > diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs > index ea5a16b8163a6..f26ca39b84d0d 100644 > --- 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: Medium] Does this explanation violate the safety invariants of the RefCounted trait? The RefCounted trait strictly requires that increments to the reference count keep the object alive, and that all instances are reference-counted. By using a no-op inc_ref and dec_ref, this example demonstrates an unsound implementation of the trait, which might lead developers to introduce use-after-free bugs if they copy this pattern into production code. > /// } > /// > /// 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: Medium] Is it safe to rely on stack-variable lifetimes here to bypass the normal reference counting expectations? ARef::from_raw and the RefCounted trait expect the object's lifetime to be handled inherently via reference counting. Relying on keeping a stack variable around longer than the ARef bypasses ARef::from_raw safety preconditions. Could this documentation lead others to incorrectly apply ARef to stack variables? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
