[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1190318781.26101.148.camel@localhost>
Date: Thu, 20 Sep 2007 13:06:21 -0700
From: Joe Perches <joe@...ches.com>
To: Urs Thuermann <urs@...ogud.escape.de>
Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
Patrick McHardy <kaber@...sh.net>,
Thomas Gleixner <tglx@...utronix.de>,
Oliver Hartkopp <oliver@...tkopp.net>,
Oliver Hartkopp <oliver.hartkopp@...kswagen.de>,
Urs Thuermann <urs.thuermann@...kswagen.de>
Subject: Re: [PATCH 2/7] CAN: Add PF_CAN core module
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...) \
do { \
if (debug & 1) \
printk(KERN_DEBUG "can-%s %s: " fmt, IDENT, __func__, ##args); \
} while (0)
#define DBG_FRAME(cf, fmt, args...) \
do { \
if (debug & 2) \
can_debug_cframe(cf, fmt, ##args); \
} while (0)
#define DBG_SKB(skb) \
do { \
if (debug & 4) \
can_debug_skb(skb); \
while (0)
> void can_debug_cframe(const char *msg, struct can_frame *cf, ...)
This prototype looks backwards to me.
msg is a printf format string.
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...) \
do { \
char buf[20]; \
int dlc = cf->can_dlc; \
if (dlc > 8) \
dlc = 8; \
if (cf->can_id & CAN_EFF_FLAG) \
sprintf(buf, "<%08X> [%X] ", cf->can_id & CAN_EFF_MASK, dlc); \
else \
sprintf(buf, "<%03X> [%X] ", cf->can_id & CAN_SFF_MASK, dlc); \
printk(KERN_DEBUG fmt, ##arg); \
print_hex_dump(buf, DUMP_PREFIX_NONE, cf->data, dlc); \
} while (0)
and
#define can_debug_skb(skb) \
do { \
pr_debug("skbuff at %p, dev: %d, proto: %04x\n", \
skb, skb->dev ? skb->dev->ifindex : -1, \
ntohs(skb->protocol)); \
pr_debug("users: %d, dataref: %d, nr_frags: %d, " \
"h,d,t,e,l: %p %+d %+d %+d, %d\n", \
atomic_read(&skb->users), \
atomic_read(&(skb_shinfo(skb)->dataref)), \
skb_shinfo(skb)->nr_frags, \
skb->head, skb->data - skb->head, \
skb->tail - skb->head, skb->end - skb->head, skb->len); \
print_hex_dump(KERN_DEBUG, "skb_head: ", DUMP_PREFIX_NONE, \
16, 1, skb->head, skb->end - skb->head); \
} while (0)
cheers, Joe
-
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
Powered by blists - more mailing lists