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

Reply via email to