Re: [PATCH v3 01/16] rust: io: register: reimplement as proc macro

[email protected]
Newsgroups org.kernel.vger.linux-pci,org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing parentheses around `#idx` in the generated register offset expression causes operator precedence inversion.
--

--- Patch [1]: [PATCH v3 01/16] rust: io: register: reimplement as proc macro ---
commit e2035dd40ab99d9c728dbbe1a208159d8a85be92
Author: Gary Guo <[email protected]>
rust: io: register: reimplement as proc macro

This commit reimplements the existing declarative macro for register allocation
as a proc macro. This change provides more helpful diagnostics for syntax
errors and allows more flexible syntax without follow-set restrictions.

> diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
> new file mode 100644
> index 0000000000000..61d076ab570a0
> --- /dev/null
> +++ b/rust/macros/io/register.rs

[ ... ]

> +        let offset = match offset {
> +            RegOffset::Fixed { offset } => quote!(#offset),
> +            RegOffset::Alias { alias } => {
> +                quote_spanned!(alias.span().resolved_at(span) =>
> +                    <#alias as ::kernel::io::register::Register>::OFFSET
> +                )
> +            }
> +            RegOffset::ElementAlias { alias, idx } => {
> +                outputs.extend(quote_spanned!(idx.span().resolved_at(span) =>
> +                    ::kernel::build_assert::static_assert!(
> +                        #idx < <#alias as ::kernel::io::register::RegisterArray>::SIZE
> +                    );
> +                ));
> +                quote_spanned!(alias.span().resolved_at(span) =>
> +                    <#alias as ::kernel::io::register::Register>::OFFSET
> +                        + #idx * <#alias as ::kernel::io::register::RegisterArray>::STRIDE

[Severity: High]
Will this cause operator precedence issues if the parsed index expression
contains addition or subtraction?

Because idx is parsed as an Expr and inserted directly into the quote block
without parentheses, complex expressions will be emitted verbatim. If a
developer uses an expression like A + B for the array index, the generated
code would expand to:

OFFSET + A + B * STRIDE

Because multiplication has higher precedence, this calculates
OFFSET + A + (B * STRIDE) instead of the intended OFFSET + ((A + B) * STRIDE).
This could result in calculating incorrect I/O memory offsets and writing to
the wrong device registers.

Could idx be wrapped in parentheses here?

> +                )
> +            }
> +        };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.