Re: [PATCH 2/7] CAN: Add PF_CAN core module

Urs Thuermann
2007-11-15T11:55:17Z

Joe Perches <joe@perches.com> writes:

> On Thu, 2007-11-15 at 08:40 +0100, Oliver Hartkopp wrote:
> > Stephen Hemminger wrote:
> > >> +#ifdef CONFIG_CAN_DEBUG_CORE
> > >> +extern void can_debug_skb(struct sk_buff *skb);
> > >> +extern void can_debug_cframe(const char *msg, struct can_frame *cframe);
> > >> +#define DBG(fmt, args...) (DBG_VAR & 1 ? printk( \
> > >> + KERN_DEBUG DBG_PREFIX ": %s: " fmt, \
> > >> + __func__, ##args) : 0)
> > >> +#define DBG_FRAME(fmt, cf) (DBG_VAR & 2 ? can_debug_cframe(fmt, cf) : 0)
> > >> +#define DBG_SKB(skb) (DBG_VAR & 4 ? can_debug_skb(skb) : 0)
> > >> +#else
> > >> +#define DBG(fmt, args...)
> > >> +#define DBG_FRAME(fmt, cf)
> > >> +#define DBG_SKB(skb)
> > >> +#endif
>
> I would prefer the more frequently used macro style:
>
> #define DBG(fmt, args...) \
> do { if (DBG_VAR & 1) printk(KERN_DEBUG DBG_PREFIX ": %s: " fmt, \
> __func__, ##args); } while (0)
>
> #define DBG_FRAME(fmt, cf) \
> do { if (DBG_VAR & 2) can_debug_cframe(fmt, cf); } while (0)
>
> #define DBG_SKB(skb) \
> do { if (DBG_VAR & 4) can_debug_skb(skb); } while (0)

I prefer our code because it is shorter (fits into one line) and can
be used anywhere where an expression is allowed compared to only where
a statement is allowed. Actually, I first had

#define DBG( ... ) ((debug & 1) && printk( ... ))

and so on, but that didn't work with can_debug_{cframe,sbk} since they
return void.

Admitted, the benefit of expr vs. statement is really negligible and
since this issue has come up several times I will change these macros
using do-while.

urs