Re: [PATCH 1/3] CodingStyle updates

Sam Ravnborg
2007-09-29T18:27:51Z

On Sat, Sep 29, 2007 at 11:01:29AM -0700, Randy Dunlap wrote:
> On Fri, 28 Sep 2007 17:32:00 -0400 Erez Zadok wrote:
>
> > 1. Updates chapter 13 (printing kernel messages) to expand on the use of
> > pr_debug()/pr_info(), what to avoid, and how to hook your debug code with
> > kernel.h.
> >
> > Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
> > ---
> > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 files changed, 86 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> > index 7f1730f..00b29e4 100644
> > --- a/Documentation/CodingStyle
> > +++ b/Documentation/CodingStyle
> > @@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided.
> > There are a number of driver model diagnostic macros in <linux/device.h>
> > which you should use to make sure messages are matched to the right device
> > and driver, and are tagged with the right level: dev_err(), dev_warn(),
> > -dev_info(), and so forth. For messages that aren't associated with a
> > -particular device, <linux/kernel.h> defines pr_debug() and pr_info().
> > +dev_info(), and so forth.
> > +
> > +A number of people often like to define their own debugging printf's,
> > +wrapping printk's in #ifdef's that get turned on only when subsystem
> > +debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please
> > +don't reinvent the wheel but use existing mechanisms. For messages that
> > +aren't associated with a particular device, <linux/kernel.h> defines
> > +pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and
> > +printk(KERN_INFO), respectively. However, to get pr_debug() to actually
> > +emit the message, you'll need to turn on DEBUG in your code, which can be
> > +done as follows in your subsystem Makefile:
> > +
> > +ifeq ($(CONFIG_WHATEVER_DEBUG),y)
> > +EXTRA_CFLAGS += -DDEBUG
> > +endif
The abvoe hurst my eyes each time I see it.
Lately I have considered extending the kbuild syntax a bit.

Introducing
ccflags-y
asflags-y

[with same functionality as the EXTRA_CFLAGS, EXTRA_AFLAGS]
would allow us to do:

ccflags-$(CONFIG_WHATEVER_DEBUG) := -DDEBUG

Much nicer IMHO.

Sam