Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Incomplete SAFETY comments for `Bitmap::from_raw` calls in unit tests. --
commit f2871aa21f86bcf804086540b9bc1b66aba9457a Author: Eliot Courtney <[email protected]> rust: bitmap: restrict bitmap length to at most i32::MAX This restricts all Bitmaps (even non-BitmapVec backed ones) to a maximum size of i32::MAX to simplify operations and match BitmapVec::MAX_LEN. It adds this requirement to the unsafe preconditions and invariants of Bitmap functions. Link: https://lore.kernel.org/[email protected] > diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs > index a43bfe0ec3dc2..df5505ec7a965 100644 > --- a/rust/kernel/bitmap.rs > +++ b/rust/kernel/bitmap.rs [ ... ] > @@ -17,24 +17,59 @@ [ ... ] > impl Bitmap { > /// Borrows a C bitmap. > /// > + /// # Panics > + /// > + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds > `i32::MAX`. > + /// > /// # Safety > /// > /// * `ptr` holds a non-null address of an initialized array of > `unsigned long` > /// that is large enough to hold `nbits` bits. > + /// * `nbits` must not exceed `i32::MAX`. > /// * the array must not be freed for the lifetime of this [`Bitmap`] > /// * concurrent access only happens through atomic operations > pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a > Bitmap { [Severity: Low] Since a new safety precondition is added here, shouldn't the SAFETY comments on existing callers be updated to explain how this condition is satisfied? For example, the test code in rust/kernel/bitmap.rs calls from_raw() but the comments only justify the array length: rust/kernel/bitmap.rs:tests::bitmap_borrow() { ... // SAFETY: `fake_c_bitmap` is an array of expected length. let b = unsafe { Bitmap::from_raw(fake_bitmap.as_ptr(), fake_bitmap_len) }; ... } rust/kernel/bitmap.rs:tests::bitmap_copy() { ... // SAFETY: `fake_c_bitmap` can be used as one-element array of expected length. let b = unsafe { Bitmap::from_raw(core::ptr::addr_of!(fake_bitmap), 8) }; ... } Should these be updated to also mention that fake_bitmap_len and 8 do not exceed i32::MAX? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
