Re: [PATCH 2/5] rust: drm: add connector abstraction
| Newsgroups | gmane.linux.drivers.devicetree,gmane.comp.video.dri.devel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] The `Connector` struct's `# Invariants` documentation incorrectly references an 'inner pointer', whereas the struct is a transparent wrapper around an `Opaque` type. - [Low] The forwarding methods `from_raw` and `as_raw` in the `Connector` abstraction lack the required `#[inline]` attribute. -- --- Patch [2]: [PATCH 2/5] rust: drm: add connector abstraction --- commit 3b78dba370077b309a3e8fb4bdf3afb4e77c1754 Author: Albert Esteve <[email protected]> rust: drm: add connector abstraction Add DRM Connector abstraction wrapping struct drm_connector. Used by the DRM panel abstractions added in the following patch. > diff --git a/rust/kernel/drm/connector.rs b/rust/kernel/drm/connector.rs > new file mode 100644 > index 0000000000000..b3b36644375dd > --- /dev/null > +++ b/rust/kernel/drm/connector.rs > @@ -0,0 +1,33 @@ [ ... ] > +/// A DRM connector (`struct drm_connector`). > +/// > +/// # Invariants > +/// > +/// The inner pointer is always a valid, non-null pointer to a `struct drm_connector`. > +#[repr(transparent)] > +pub struct Connector(Opaque<bindings::drm_connector>); [Severity: Low] this isn't a bug, but the invariant documentation references an "inner pointer". Since the struct directly wraps an Opaque type rather than holding a pointer itself, should this be updated to reflect that? > + > +impl Connector { > + /// Creates a reference to a [`Connector`] from a raw pointer. > + /// > + /// # Safety > + /// > + /// `ptr` must be a valid, non-null pointer to a `struct drm_connector` that > + /// remains valid for at least the lifetime `'a`. > + pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_connector) -> &'a Self { [Severity: Low] this isn't a bug, but as a small forwarding function for an abstraction, should this method be annotated with #[inline]? > + // SAFETY: Caller guarantees `ptr` is valid and lives for `'a`. > + unsafe { &*ptr.cast() } > + } > + > + /// Returns the raw pointer to the underlying `struct drm_connector`. > + pub fn as_raw(&self) -> *mut bindings::drm_connector { [Severity: Low] this isn't a bug, but should this method also be annotated with #[inline] since it merely forwards to the underlying binding call? > + self.0.get() > + } > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org?part=2