Re: [PATCH v3 01/16] rust: io: register: reimplement as proc macro
| 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