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

Thomas Gleixner
2007-09-20T20:27:50Z

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