Date:	Tue, 08 Apr 2008 14:02:29 -0700
From:	Joe Perches <joe@...ches.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	David Miller <davem@...emloft.net>, johannes@...solutions.net,
	harvey.harrison@...il.com, netdev@...r.kernel.org
Subject: Re: New sparse warning in net/mac80211/debugfs_sta.c

On Tue, 2008-04-08 at 22:18 +0200, Patrick McHardy wrote:
> Joe Perches wrote:
> > On Mon, 2008-02-25 at 12:47 +0100, Patrick McHardy wrote: 
> >> It would be good if Joe could go through the remaining print_mac users
> >> and convert the remaining unintended function calls in fastpaths back
> >> to MAC_FMT. Grepping for "start_xmit" in commit 0795af5729b shows that
> >> at least 10 hard_start_xmit functions are affected and I expect that
> >> some of the changes in the wireless code affect fastpaths as well.
> > I don't mind doing that, as calling print_mac in these fastpaths in
> > unintentional and undesirable.
> Unfortunately the current tree still includes all the fallout,
> are you planning on cleaning this up again any time soon?
> I've attached a codiff of a tree with and without this change
> (might not include all drivers, but I think I enabled all that
> build on x86_64). The _probe and _init_one functions should
> be harmless, but there are lots of functions that look like
> they would prefer to avoid useless overhead. A small sample:
> 
> drivers/net/starfire.c:
>    netdev_poll       |  +50
> 
> drivers/net/tokenring/olympic.c:
>    olympic_interrupt |  +10

[]

I also think this should be cleaned up before 2.6.25 is released.

I think that the changes to pr_debug, dev_dbg, and dev_vdbg
to use an "if (0) printk" macro rather than an empty inline
I posted a few times without any reply or comment should work
for most all cases.

These changes should allow gcc to eliminate unused functions
called as arguments to those debugging logging functions
while maintaining the printf argument validation.

I'll check out codiff as I haven't used it.
Is this the latest codiff tool?
http://git.kernel.org/?p=linux/kernel/git/acme/pahole.git;a=summary

cheers, Joe

Here is the patch again:

Signed-off-by: Joe Perches <joe@...ches.com>

diff --git a/include/linux/device.h b/include/linux/device.h
index 2258d89..12bb248 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -608,21 +608,21 @@ extern const char *dev_driver_string(struct device *dev);
 #define dev_dbg(dev, format, arg...)		\
 	dev_printk(KERN_DEBUG , dev , format , ## arg)
 #else
-static inline int __attribute__ ((format (printf, 2, 3)))
-dev_dbg(struct device *dev, const char *fmt, ...)
-{
-	return 0;
-}
+#define dev_dbg(dev, format, arg...)				\
+do {								\
+	if (0)							\
+		dev_printk(KERN_DEBUG , dev , format , ## arg);	\
+} while (0)
 #endif
 
 #ifdef VERBOSE_DEBUG
 #define dev_vdbg	dev_dbg
 #else
-static inline int __attribute__ ((format (printf, 2, 3)))
-dev_vdbg(struct device *dev, const char *fmt, ...)
-{
-	return 0;
-}
+#define dev_vdbg(dev, format, arg...)				\
+do {								\
+	if (0)							\
+		dev_printk(KERN_DEBUG , dev , format , ## arg);	\
+} while (0)
 #endif
 
 /* Create alias, so I can be autoloaded. */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2df44e7..80be070 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -293,10 +293,11 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 #define pr_debug(fmt, arg...) \
 	printk(KERN_DEBUG fmt, ##arg)
 #else
-static inline int __attribute__ ((format (printf, 1, 2))) pr_debug(const char * fmt, ...)
-{
-	return 0;
-}
+#define pr_debug(fmt, arg...)			\
+do {						\
+	if (0)					\
+		printk(KERN_DEBUG fmt, ##arg);	\
+} while (0)
 #endif
 
 /*


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists