Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The `Page::from_raw` method silently bypasses the documented type invariants of the `Page` struct and lacks an `// INVARIANT:` comment. --
commit 224010a6541ae8f534f1577d76a1acd3e9bb5d34 Author: Andreas Hindborg <[email protected]> rust: page: add `from_raw()` This commit adds a new from_raw() method to the Page struct. It enables the creation of a shared &Page reference directly from a raw struct page C pointer. > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs > index 844c75e541345..d56ae597f6926 100644 > --- a/rust/kernel/page.rs > +++ b/rust/kernel/page.rs [ ... ] > @@ -214,6 +214,18 @@ pub fn nid(&self) -> i32 { > unsafe { bindings::page_to_nid(self.as_ptr()) } > } > > + /// Create a `&Page` from a raw `struct page` pointer. > + /// > + /// # Safety > + /// > + /// `ptr` must be convertible to a shared reference with a lifetime of > `'a`. > + #[inline] > + pub unsafe fn from_raw<'a>(ptr: *const bindings::page) -> &'a Self { > + // SAFETY: By function safety requirements, `ptr` is not null and is > convertible to a shared > + // reference. > + unsafe { &*ptr.cast() } [Severity: Low] This isn't a bug, but should this unsafe block include an `// INVARIANT:` comment to justify the type invariants of the Page struct? The Page struct has an `# Invariants` documentation section stating that it has ownership over the page. While that ownership invariant might be an obsolete leftover, creating a &Page reference here conceptually asserts that the referenced memory satisfies this type invariant. Could we add the required `// INVARIANT:` comment or update the outdated type invariant documentation to clarify this? > + } > + > /// Runs a piece of code with this page mapped to an address. > /// > /// The page is unmapped when this call returns. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
