netdev - Re: [PATCH] SUNRPC: Mark buffer used for debug printks with __maybe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 20 Feb 2008 12:41:45 -0500
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Joe Perches <joe@...ches.com>
Cc:	Patrick McHardy <kaber@...sh.net>,
	Pavel Emelyanov <xemul@...nvz.org>,
	"J. Bruce Fields" <bfields@...i.umich.edu>,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH] SUNRPC: Mark buffer used for debug printks
	with	__maybe_unused


On Wed, 2008-02-20 at 09:23 -0800, Joe Perches wrote:
> On Wed, 2008-02-20 at 12:02 -0500, Trond Myklebust wrote:
> > > #ifdef DEBUG
> > > #define some_print_wrapper(fmt, arg...) \
> > >         do { if (0) printk(KERN_DEBUG fmt, ##arg); } while (0)
> > > #else
> > > #define some_print_wrapper(fmt, arg...) \
> > > 	printk(KERN_DEBUG fmt, ##arg)
> > > #endif
> > Have you actually read include/linux/sunrpc/debug.h?
> 
> Yes, I have.
> 
> What's there:
> 
> #define dfprintk(fac, args...) do ; while (0)
> 
> vs what's suggested:
> 
> #define dfprintk(fac, args...) \
> 	do  { if (0) printk(##args); } while (0);
> 
> No argument verification is done to args
> 
> There has been code that fails to compile with -DDEBUG when
> the code use two different #ifdef DEBUG #else macros.
> I think some of the USB code was reworked because of that.
> 
> The extra verification is just a guard against bad arguments
> when compiled normally.  It's similar to what's done in kernel.h
> pr_debug without the __attribute__((format(printf,x,y))) so
> that calls made as arguments to functions aren't called
> unnecessarily.

RPC_DEBUG is on by default if SYSCTL is compiled in, so it is not as if
we're having any trouble typechecking the arguments. In fact, most of
the major distros also tend to enable RPC_DEBUG, since it does come in
handy when their customers report rpc/nfs problems.

As you can see, there is already a macro RPC_IFDEBUG to deal with simple
code that depends on whether or not RPC_DEBUG is set. Using that will
make it obvious to a reader exactly when the declaration will be
optimised away, and why, without compromising gcc's ability to warn us
if it were to be unused due to some future code change.


--
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