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 <[email protected]> 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/[email protected]?part=3
