Re: [PATCH v21 3/4] rust: faux: Allow retrieving a bound Device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing `// INVARIANT:` comment in `Registration::new` to justify the newly added struct invariant.
- [Low] Documentation typo: states the object is 'bound to a device' instead of 'bound to a driver'.
--

commit 53b3c6bd4c01a202375ed187c918196f25a37895
Author: Lyude Paul <lyude@redhat.com>

rust: faux: Allow retrieving a bound Device

This commit updates faux::Registration to implement AsRef for a
device::Device<device::Bound>. It relies on the registration object being
proof that a driver is bound to the device.

> diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs
> index 43b4974f48cd2..36c92ae2943c0 100644
> --- a/rust/kernel/faux.rs
> +++ b/rust/kernel/faux.rs
> @@ -25,7 +25,8 @@
>  ///
>  /// # Invariants
>  ///
> -/// `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`].
> +/// - `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`].
> +/// - This object is proof that the object described by this `Registration` is bound to a device.

[Severity: Low]
This isn't a bug, but there appears to be a typo in the documentation. The
invariant states the object is bound to a "device", but the commit message
and the code (device::Device<device::Bound>) indicate it should be bound
to a "driver". Should this be updated to say "bound to a driver"?

[Severity: Low]
This isn't a bug, but since a new invariant was added here, does it need to
be explicitly justified when the struct is constructed?

According to the subsystem guidelines, constructing a struct with an
# Invariants section requires an // INVARIANT: comment explaining why the
invariants are satisfied.

Looking at Registration::new():

    // The above function will return either a valid device, or NULL on failure
    // INVARIANT: The device will remain registered until faux_device_destroy() is called, which
    // happens in our Drop implementation.
    Ok(Self(NonNull::new(dev).ok_or(ENODEV)?))

Could we add a comment here justifying the new invariant about the driver
being bound?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612194436.585385-1-lyude@redhat.com?part=3