Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The patch papers over a compilation error in `pr_debug_ratelimited` 
by modifying `DEFINE_DYNAMIC_DEBUG_METADATA_CLS` to silently discard variadic 
arguments, instead of fixing the root cause API inconsistency where 
`pr_fmt(fmt)` is incorrectly passed to the descriptor.
--

commit 187c89f348922db282033311e2fc8e7a59f688c1
Author: Philipp Hahn <[email protected]>

dyndbg: Ignore additional arguments from pr_fmt

This commit adds a variadic arguments slurping parameter to
DEFINE_DYNAMIC_DEBUG_METADATA_CLS to avoid a compile error when
pr_debug_ratelimited is used with a pr_fmt that includes additional arguments.
It also updates lib/test_dynamic_debug.c to test this behavior.

> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 32a34a80672b3..39de59c8bac2c 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -364,7 +364,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
>               dump_stack();                                   \
>  }
>  
> -#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)    \
> +#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt, ...)       \
>       static struct _ddebug  __aligned(8)                     \
>       __section("__dyndbg_descs") name = {                    \
>               .modname = DDEBUG_MODNAME,                      \

[Severity: Medium]
Does adding the variadic arguments here and silently discarding them corrupt
the stored format string in the dynamic debug control file?

If a module defines pr_fmt(fmt) with additional arguments (such as __func__),
DEFINE_DYNAMIC_DEBUG_METADATA_CLS will capture a mangled format string like
"test_dd_submod: %s " fmt while discarding the format arguments. This leaves
unresolvable %s specifiers in the control file.

This change also makes the stored format string inconsistent with pr_debug,
dev_dbg_ratelimited, and net_dbg_ratelimited, which correctly use fmt without
the pr_fmt prefix.

Instead of discarding the arguments here, should the root cause in
include/linux/printk.h be fixed so that pr_debug_ratelimited passes fmt to
DEFINE_DYNAMIC_DEBUG_METADATA rather than pr_fmt(fmt)?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=36

Reply via email to