Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] The `inc_ref` function body for `Chip<T>` still uses the undefined `self` identifier instead of the new `obj` parameter. - [Low] Safety comments in multiple files still reference `self` instead of the renamed `obj` parameter. --
commit 26c2b1b363c343fdcaf27bbc75d1dde6b5b4dbfc Author: Trevor Chan <[email protected]> rust: aref: make AlwaysRefCounted::inc_ref an associated function This commit changes inc_ref from a method to an associated function to prevent accidental calls. All implementors of AlwaysRefCounted are updated to reflect this change. Link: https://github.com/Rust-for-Linux/linux/issues/1177 > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs > index 4764d7b68f2a7..c955cbd884b8b 100644 > --- a/rust/kernel/mm.rs > +++ b/rust/kernel/mm.rs > @@ -57,9 +57,9 @@ unsafe impl Sync for Mm {} > // SAFETY: By the type invariants, this type is always refcounted. > unsafe impl AlwaysRefCounted for Mm { > #[inline] > - fn inc_ref(&self) { > + fn inc_ref(obj: &Self) { > // SAFETY: The pointer is valid since self is a reference. > - unsafe { bindings::mmgrab(self.as_raw()) }; > + unsafe { bindings::mmgrab(obj.as_raw()) }; [Severity: Low] This isn't a bug, but should the SAFETY comment be updated to say "since obj is a reference" instead of "self"? [ ... ] > diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs > index 6c9d667009ef7..c95b442a972e6 100644 > --- a/rust/kernel/pwm.rs > +++ b/rust/kernel/pwm.rs > @@ -631,7 +631,7 @@ pub fn new<'a>( > // SAFETY: Implements refcounting for `Chip` using the embedded `struct > device`. > unsafe impl<T: PwmOps> AlwaysRefCounted for Chip<T> { > #[inline] > - fn inc_ref(&self) { > + fn inc_ref(obj: &Self) { > // SAFETY: `self.0.get()` points to a valid `pwm_chip` because > `self` exists. > // The embedded `dev` is valid. `get_device` increments its refcount. > unsafe { bindings::get_device(&raw mut (*self.0.get()).dev) }; [Severity: Low] Will this cause a compilation error? The function parameter was renamed to obj, but the function body still references self in self.0.get(). This isn't a bug on its own, but should the SAFETY comment also be updated to reference obj instead of self? [ ... ] > diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs > index 7aff0c82d0afc..c039059c18913 100644 > --- a/rust/kernel/usb.rs > +++ b/rust/kernel/usb.rs > @@ -393,11 +393,11 @@ fn as_ref(&self) -> &Device { > > // SAFETY: Instances of `Interface` are always reference-counted. > unsafe impl AlwaysRefCounted for Interface { > - fn inc_ref(&self) { > + fn inc_ref(obj: &Self) { > // SAFETY: The invariants of `Interface` guarantee that > `self.as_raw()` > // returns a valid `struct usb_interface` pointer, for which we will > // acquire a new refcount. > - unsafe { bindings::usb_get_intf(self.as_raw()) }; > + unsafe { bindings::usb_get_intf(obj.as_raw()) }; [Severity: Low] This isn't a bug, but should the SAFETY comment here also be updated to reference obj.as_raw() instead of self.as_raw()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
