From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8370444C4FA for ; Sun, 11 Oct 2026 13:01:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791723685; cv=none; b=VXrF9joBSaCfJe64M8df0uMIXHwISshFrv9iNRQOXs/umk5y4/f7aupr+TWpr7YX9zJMiEgxsYSnOdstS/gE6xBgsDf+ojpaDwdRFhhVOEgFRcnqlNEImncOW5wKK/tqJjKYXGFX1bKtdQ4G9vTDmgor6NtZlVrE/I9yiBYbSSo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791723685; c=relaxed/simple; bh=mHMq/cxdf7xSS8tEqwkAucejcch2sqPrezATaxQBz9Y=; h=Message-ID:From:Subject:To:Cc:In-Reply-To:References:Content-Type: Date; b=AdrNQJTvgAGbUW13Ts/LSl6v4SL7M6jJZcotbjCO2cOnHrRC4HV6s88lsbnB8gUGT/eimhOyiksq/6J2XCc80mGxr8ZlctdzCa8D8jBUYLCJAk3+da1dtvP+5zkcVqd7T5h+p6kExMIie0StVvCKfQk62O0Rm2vZ74pGpXQ9tlo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=nV7hBwJt; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="nV7hBwJt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 878BE1F0089B; Sun, 11 Oct 2026 13:01:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1791723679; bh=JSxTBrTy5fc3bjoMB/CuMsWn86+Rh7+1ABIrxsj0FIQ=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=nV7hBwJtJdB8HBSj19qofzVCuvY1mC7uZyriR95j9IyZazIp6tVuLne1/d81OUyd1 sSiGnWvexwcaLKAj2XxDDcKKsoukQQ3yZyeKCr0VKq37X+B8x11twHpWp2Ndgb/QcO 5fEkTz78VScJRAn3uV4+jglz4nmjs+evA+yBqAcx0uPqEbFwWyH8TnrSmpSfWXjzMN 8IlfW1WoW73PlPRHOdUKekSbDN4MeOfIT4PrhzU4Qmf0dVDc1rGn+bfFZZCJdf7MB8 jJM/5WWJGuDoa9edUIUx5Dgaz+m0meSqT5K1jLebOQvme6kfkbyVfv78FewemTTMW2 mZLbo02/OcLuA== Message-ID: From: sashiko-bot@kernel.org Subject: Re: [PATCH RFC 09/11] rust: print: enable dynamic debug for pr_debug!() Reply-To: sashiko-reviews@lists.linux.dev To: =?utf-8?b?VGhvbWFzIFdlacOfc2NodWg=?= Cc: linux-modules@vger.kernel.org, gary@garyguo.net, "Petr Pavlu" , ojeda@kernel.org In-Reply-To: <20261011-rust-dyndbg-v1-9-832e4039b71f@weissschuh.net> References: <20261011-rust-dyndbg-v1-0-832e4039b71f@weissschuh.net> <20261011-rust-dyndbg-v1-9-832e4039b71f@weissschuh.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Sun, 11 Oct 2026 13:01:19 +0000 Precedence: bulk X-Mailing-List: linux-modules@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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 b= lock - [High] rust: print: pr_debug() argument evaluation inside closure breaks = control flow - [Medium] rust: dynamic_debug: __format_string() rejects macro-generated f= ormat strings -- commit 0f009656f0da58d6ed039063d8a722503a6cbfbf Author: Thomas Wei=C3=9Fschuh 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 =3D "print")] > macro_rules! pr_debug ( > ($($arg:tt)*) =3D> ( [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 expressi= ons. 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 _ =3D pr_debug!(...);, won't the Rust parser fail expecting a single expression, returning an error about ignoring the semico= lon and following tokens? > + // TODO: Switch to cfg_select!() when that is usable. > + #[cfg(any(CONFIG_DYNAMIC_DEBUG, all(CONFIG_DYNAMIC_DEBUG_CORE, d= ebug_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) =3D> { $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 patt= ern 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 flo= w? 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, f= alse, $($arg)*) > + }); > + > + #[cfg(not(any(CONFIG_DYNAMIC_DEBUG, all(CONFIG_DYNAMIC_DEBUG_COR= E, debug_assertions))))] > if cfg!(debug_assertions) { > $crate::print_macro!($crate::print::format_strings::DEBUG, f= alse, $($arg)*) > } > ) --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261011-rust-dyndb= g-v1-0-832e4039b71f@weissschuh.net?part=3D9