Re: [PATCH v6] rust: aref: make `AlwaysRefCounted::inc_ref` an associated function

[sashiko-bot@kernel.org]

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 <trev@trevrosa.dev>

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/20260628095132.47753-1-trev@trevrosa.dev?part=1