Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] rust: print: pr_debug() expands to multiple statements without a block - [High] rust: print: pr_debug() argument evaluation inside closure breaks control flow - [Medium] rust: dynamic_debug: __format_string() rejects macro-generated format strings --
commit 0f009656f0da58d6ed039063d8a722503a6cbfbf Author: Thomas Weißschuh <[email protected]> rust: print: enable dynamic debug for pr_debug!() This commit wires up dynamic debug support for the pr_debug!() macro. > diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs > index 9ce52b9fa69f0..9a0870c2df940 100644 > --- a/rust/kernel/print.rs > +++ b/rust/kernel/print.rs [ ... ] > @@ -392,6 +391,13 @@ macro_rules! pr_info ( > #[doc(alias = "print")] > macro_rules! pr_debug ( > ($($arg:tt)*) => ( [Severity: High] Does this expansion cause syntax errors when pr_debug!() is used in an expression context? The pr_debug!() macro is documented to mimic std::print!(), which evaluates to () and can be used in expression positions like match arms or tail expressions. Because this macro expands using ( ... ) instead of a { ... } block, it exposes multiple #[cfg(...)] gated items and a trailing semicolon directly to the call site's AST. If a driver calls let _ = pr_debug!(...);, won't the Rust parser fail expecting a single expression, returning an error about ignoring the semicolon and following tokens? > + // TODO: Switch to cfg_select!() when that is usable. > + #[cfg(any(CONFIG_DYNAMIC_DEBUG, all(CONFIG_DYNAMIC_DEBUG_CORE, > debug_assertions)))] > + $crate::dynamic_func_call!($crate::__format_string!($($arg)+), || { [Severity: Medium] Will this break if the format string is generated by another macro? The user-provided arguments are passed to __format_string!(): rust/kernel/dynamic_debug.rs: macro_rules! __format_string { ($fmt:literal) => { $fmt }; ... Because __format_string!() strictly expects a literal token, if a driver calls pr_debug!(concat!("a", "b")), won't it fail to match the literal pattern and cause a compile error? Standard Rust formatting macros typically allow macro-generated format strings. [Severity: High] Can evaluating the arguments inside this closure silently break control flow? The user-provided arguments $($arg)* are expanded inside the || { ... } closure. If a caller uses a control-flow operator in their arguments, such as pr_debug!("Result: {}", do_something()?);, won't the ? operator attempt to return from the closure instead of the caller's function? This would either cause a type mismatch error because the closure returns (), or silently return from the print closure while the outer function continues executing unexpectedly. > + $crate::print_macro!($crate::print::format_strings::DEBUG, > false, $($arg)*) > + }); > + > + #[cfg(not(any(CONFIG_DYNAMIC_DEBUG, all(CONFIG_DYNAMIC_DEBUG_CORE, > debug_assertions))))] > if cfg!(debug_assertions) { > $crate::print_macro!($crate::print::format_strings::DEBUG, > false, $($arg)*) > } > ) -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
