Re: [PATCH 2/3] net/netfilter: Convert printk uses to pr_<level>

Re: [PATCH 2/3] net/netfilter: Convert printk uses to pr_<level>

[Joe Perches]

On Sat, 2009-06-13 at 12:45 +0200, Patrick McHardy wrote:
> Joe Perches wrote:
> > Remove function names from format strings
> > Add #define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
> 
> For non-debugging messages that should provide enough information
> to make sense of them without function names, this change does not
> make much sense in my opinion.
> 
> Instead of a half-way readable message, we now would get:
> 
> nf_conntrack: nf_conntrack_acct_init: CONFIG_NF_CT_ACCT is deprecated ...
> 
> which is not an improvement at all. So unless we can limit this
> to debugging message, I'll pass on this change.

Good point.

Maybe there should be a #define pr_dbg_fmt(fmt) <foo>

Perhaps:

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 883cd44..adda97d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -360,6 +360,10 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 #define pr_fmt(fmt) fmt
 #endif
 
+#ifndef pr_dbg_fmt
+#define pr_dbg_fmt(fmt) pr_fmt(fmt)
+#endif
+
 #define pr_emerg(fmt, ...) \
         printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_alert(fmt, ...) \
@@ -380,16 +384,16 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 /* pr_devel() should produce zero code unless DEBUG is defined */
 #ifdef DEBUG
 #define pr_devel(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+	printk(KERN_DEBUG pr_dbg_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_devel(fmt, ...) \
-	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
+	({ if (0) printk(KERN_DEBUG pr_dbg_fmt(fmt), ##__VA_ARGS__); 0; })
 #endif
 
 /* If you are writing a driver, please use dev_dbg instead */
 #if defined(DEBUG)
 #define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+	printk(KERN_DEBUG pr_dbg_fmt(fmt), ##__VA_ARGS__)
 #elif defined(CONFIG_DYNAMIC_DEBUG)
 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
 #define pr_debug(fmt, ...) do { \
@@ -397,7 +401,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 	} while (0)
 #else
 #define pr_debug(fmt, ...) \
-	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
+	({ if (0) printk(KERN_DEBUG pr_dbg_fmt(fmt), ##__VA_ARGS__); 0; })
 #endif
 
 /*