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

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 2007-09-20 at 13:06 -0700, Joe Perches wrote:
> On Thu, 2007-09-20 at 20:43 +0200, Urs Thuermann wrote:
> > +#define DBG(...)       (debug & 1 ? \
> > +                              (printk(KERN_DEBUG "can-%s %s: ", \
> > +                               IDENT, __func__), printk(args)) : 0)
> > +#define DBG_FRAME(args...) (debug & 2 ? can_debug_cframe(args) : 0)
> > +#define DBG_SKB(skb)       (debug & 4 ? can_debug_skb(skb) : 0)
> > +#else
> > +#define DBG(args...)
> > +#define DBG_FRAME(args...)
> > +#define DBG_SKB(skb)
> > +#endif
> 
> Shouldn't these be like the more common
> 
> #define DBG(fmt, args...) \

> I'd prefer something like this, which removes the unnecessary
> kmalloc/kfree pairs or the equivalent conversions to functions.
> 
> #define can_debug_cframe(cf, fmt, arg...) \

That's even worse. If we want to have something better than the DBG
macros, then please convert the whole stuff into inline functions.

#ifdef DEBUG
static inline void debug_skb(struct sk_buff *skb)
{
	if (debug & 4)
		...
}
#else
static inline debug_skb(struct sk_buff *skb) {}
#endif

Thanks,

	tglx