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

From: Joe Perches <[email-masked]>
Date: Fri, 21 Sep 2007 09:58:48 -0700

On Fri, 2007-09-21 at 12:35 +0200, Urs Thuermann wrote:
> I didn't find a way with gcc-2.95 to make the format
> string a separate macro argument (which I also wanted).

The old 2.x GCC workaround was to use
	#define DBG(fmt, arg) printk(fmt , ## arg)
adding a space before the last comma.

> I use "do { ... } while(0)" only for statements, not for expressions.
> But I could have used the && instead of ?: operator.  I don't think
> the "do { ... } while(0)" looks nicer or has any other advantage.

It's more linux convention.
It allows the macro to be used in if-else constructs.

> > > void can_debug_cframe(const char *msg, struct can_frame *cf, ...)
> > This prototype looks backwards to me.
> You mean the order or `msg' and `cf'?

Yes, I believe the can_frame* should be the first argument.

> Ah, I didn't know print_hex_dump().  That looks nicer.  But as Thomas
> mentioned, we shouldn't convert these functions into macros.

The first print_hex_dump should actually be print_hex_dump_bytes.

I was typing what I hoped was a readable example.
I hope you do convert to functions, but not to inline functions.

cheers, Joe