Re: Tiny error in printk output for clocksource : a3:<6>Time: acpi_pm clocksource has been installed.

[Denis Vlasenko]

On Thursday 28 September 2006 18:08, Joe Perches wrote:
> On Thu, 2006-09-28 at 12:56 +0200, Denis Vlasenko wrote:
> > \#define MACSTR "%02X:%02X:%02X:%02X:%02X:%02X"
> > #define MAC(bytevector) \
> >         ((unsigned char *)bytevector)[0], \
> >         ((unsigned char *)bytevector)[1], \
> >         ((unsigned char *)bytevector)[2], \
> >         ((unsigned char *)bytevector)[3], \
> >         ((unsigned char *)bytevector)[4], \
> >         ((unsigned char *)bytevector)[5]
> 
> This is similar to the 802.11 way.
> 802.11 uses MAC_FMT and MAC_ARG.
> I think a common style is preferable.
> 
> It's fine, but it increases the size of kernel image
> by up to ~100K.  Using a common function, a stack
> automatic and "%s" in the printk decreases the size
> of the kernel. 

You deleted part of my message where I show exactly that:
a common function, which handles 80% of usage cases.

You are trying to cover all possible cases with this monstrosity:

extern char *__dev_addr6_fmt(char* buf, const unsigned char *addr);
#define DEV_ADDR6_FMT "%s" /* expands to: "FF:FF:FF:FF:FF:FF" */
#define DEV_ADDR6_BUF char __dev_addr6_buf[sizeof("FF:FF:FF:FF:FF:FF")]
#define DEV_ADDR6(addr) __dev_addr6_fmt(__dev_addr6_buf,(const unsigned char*)addr)
#define DEV_ADDR6_BUF_2 char __dev_addr6_buf_2[sizeof("FF:FF:FF:FF:FF:FF")]
#define DEV_ADDR6_2(addr) __dev_addr6_fmt(__dev_addr6_buf_2,(const unsigned char*)addr)
#define DEV_ADDR6_BUF_3 char __dev_addr6_buf_3[sizeof("FF:FF:FF:FF:FF:FF")]
#define DEV_ADDR6_3(addr) __dev_addr6_fmt(__dev_addr6_buf_3,(const unsigned char*)addr)
#define DEV_ADDR6_BUF_4 char __dev_addr6_buf_4[sizeof("FF:FF:FF:FF:FF:FF")]
#define DEV_ADDR6_4(addr) __dev_addr6_fmt(__dev_addr6_buf_4,(const unsigned char*)addr)

Usage:

DEV_ADDR6_BUF;
...
printk(", h/w address " DEV_ADDR6_FMT "\n", DEV_ADDR6(dev->dev_addr));

Why don't you use a parameter for DEV_ADDR6{_BUF}? DEV_ADDR6_BUF(var_name).
DEV_ADDR6(var_name, addr). That would be less cryptic.

In my case, it's just

print_mac(", h/w address ", dev->dev_addr, "\n");

Actually, I think it makes sense to have both: yours for complicated
cases (printk with lots of other %something) and mine for simple ones
(no local variable, smaller code).

> Strictly, not all MAC addresses are 6 byte.
> 
> Maybe all the Ethernet/TR addresses should use the
> IEEE EUI48 designation?  That feels a bit like the
> KiB/KB distinction, but it is technically correct.
> 
> Would a patch with an DEV6_ADDR->EUI48 substitution
> be acceptable?

Maybe. Doesn't look obvious, but if it is in standards...
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/