Re: [PATCH v7 04/10] rust: sizes: implement SizeConstants for Alignment
| Newsgroups | org.freedesktop.lists.dri-devel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Implementing SizeConstants for Alignment logically conflates memory sizes with alignment constraints, preventing the definition of valid non-power-of-two sizes. -- --- Patch [4]: [PATCH v7 04/10] rust: sizes: implement SizeConstants for Alignment --- commit 3249458e082cdf9dedef6947c3ab8e2d8e84bcc1 Author: Eliot Courtney <[email protected]> rust: sizes: implement SizeConstants for Alignment This commit implements the SizeConstants trait for Alignment and adds more size constants mirroring include/linux/sizes.h. This allows constructing alignments using size constants like Alignment::SZ_8. Link: https://lore.kernel.org/an4xDp29VX8Am0uR@yury > diff --git a/rust/kernel/sizes.rs b/rust/kernel/sizes.rs > index 521b2b38bfe77..825d6b5669524 100644 > --- a/rust/kernel/sizes.rs > +++ b/rust/kernel/sizes.rs [ ... ] > @@ -117,6 +152,16 @@ pub trait SizeConstants { > > (@impls [] $($(#[$meta:meta])* $name:ident,)*) => {}; > > + (@impl_alignment $($(#[$meta:meta])* $name:ident,)*) => { > + impl SizeConstants for crate::ptr::Alignment { > + $( > + $(#[$meta])* > + // A non-power-of-two constant will fail the build here if used. > + const $name: Self = crate::ptr::Alignment::new_checked(self::$name).unwrap(); [Severity: Medium] Does implementing SizeConstants for Alignment logically conflate memory sizes with alignment constraints? If a developer later adds a valid non-power-of-two size (like SZ_3M from include/linux/sizes.h) to the define_sizes! macro, won't it cause a mandatory compile-time panic here because Alignment::new_checked() requires a power of two? This seems like it would prevent developers from adding legitimate non-power-of-two size constants to the macro in the future, limiting the ability to fully mirror the C header as stated in the commit message. > + )* > + } > + }; > + > (@impls [$first:ty $(, $rest:ty)*] $($(#[$meta:meta])* $name:ident,)*) => { -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4