All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	dri-devel@lists.freedesktop.org, ojeda@kernel.org
Subject: Re: [PATCH v7 04/10] rust: sizes: implement SizeConstants for Alignment
Date: Mon, 17 Aug 2026 07:12:34 +0000	[thread overview]
Message-ID: <20260817071235.163A41F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260817-chid-v7-4-a5872e64d8f4@nvidia.com>

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 <ecourtney@nvidia.com>

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/20260817-chid-v7-0-a5872e64d8f4@nvidia.com?part=4

  reply	other threads:[~2026-08-17  7:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  7:04 [PATCH v7 00/10] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 01/10] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-17 10:29   ` Gary Guo
2026-08-17  7:04 ` [PATCH v7 02/10] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 03/10] rust: num: add nz! macro for compile time NonZero values Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 04/10] rust: sizes: implement SizeConstants for Alignment Eliot Courtney
2026-08-17  7:12   ` sashiko-bot [this message]
2026-08-17  7:04 ` [PATCH v7 05/10] rust: bitmap: add contiguous area operations Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 06/10] rust: id_pool: take a NonZero capacity in with_capacity Eliot Courtney
2026-08-17  7:12   ` sashiko-bot
2026-08-17  7:04 ` [PATCH v7 07/10] rust: id_pool: add contiguous ID reservation Eliot Courtney
2026-08-17  7:14   ` sashiko-bot
2026-08-17  7:04 ` [PATCH v7 08/10] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 09/10] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 10/10] rust: use Alignment size constants Eliot Courtney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260817071235.163A41F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=ojeda@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.