On 12/03/2015 05:45 PM, Aaron Conole wrote:
> Currently, pr_debug and pr_devel will not elide function call arguments
> appearing in calls to no_printk for these macros. This is because all
> side effects must be honored before proceeding to the 0-value assignment
> in no_printk.
>
> The behavior is contrary to documentation found in the CodingStyle and
> header file where these functions are declared.
>
> This patch corrects that behavior by shunting out the call to no_printk
> completely. The format string is still checked by gcc for correctness, but
> no code seems to be emitted in common cases.
>
> fixes commit 5264f2f75d86 ("include/linux/printk.h: use and neaten
> no_printk")
>
> Signed-off-by: Aaron Conole <[email protected]>
> Reported-by: Dmitry Vyukov <[email protected]>
> Cc: Joe Perches <[email protected]>
I think we should just convert no_printk() to not emit anything. This
will avoid us adding unwrapped calls to 'no_printk()' in the future, and
I think makes the code more readable. Based on Joe's previous
'eliminated_printk()' thing. IE:
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9729565..58632bf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -108,11 +108,11 @@ struct va_format {
* Dummy printk for disabled debugging statements to use whilst maintaining
* gcc's format and side-effect checking.
*/
-static inline __printf(1, 2)
-int no_printk(const char *fmt, ...)
-{
- return 0;
-}
+#define no_printk(fmt, ...) \
+do { \
+ if (0) \
+ printk(fmt, ##__VA_ARGS__); \
+} while (0)
#ifdef CONFIG_EARLY_PRINTK
extern asmlinkage __printf(1, 2)
Thanks,
-Jason
>
> ---
> include/linux/printk.h | 42 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 9729565..87dfdd2 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -270,8 +270,12 @@ extern asmlinkage void dump_stack(void) __cold;
> #define pr_devel(fmt, ...) \
> printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_devel(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_devel(fmt, ...) \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> #include <linux/dynamic_debug.h>
> @@ -286,7 +290,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_debug(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /*
> @@ -341,7 +349,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_devel_once(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /* If you are writing a driver, please use dev_dbg instead */
> @@ -350,7 +362,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_debug_once(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /*
> @@ -392,8 +408,12 @@ extern asmlinkage void dump_stack(void) __cold;
> #define pr_devel_ratelimited(fmt, ...)
> \
> printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_devel_ratelimited(fmt, ...)
> \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_devel_ratelimited(fmt, ...) \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /* If you are writing a driver, please use dev_dbg instead */
> @@ -413,8 +433,12 @@ do {
> \
> #define pr_debug_ratelimited(fmt, ...)
> \
> printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_debug_ratelimited(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_debug_ratelimited(fmt, ...)
> \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> extern const struct file_operations kmsg_fops;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
