[PATCH] add %pM printf format specifier

[PATCH] add %pM printf format specifier

[Johannes Berg]

This adds a new printf format specifier for the kernel, %pM,
to be used to print out MAC addresses. This has advantages
over the current print_mac scheme:
 * no need for DECLARE_MAC_BUF
 * can be used safely in statements that might be compiled
   out without the print_mac call staying.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 lib/vsprintf.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--- everything.orig/lib/vsprintf.c	2008-10-25 01:21:54.000000000 +0200
+++ everything/lib/vsprintf.c	2008-10-25 01:24:15.000000000 +0200
@@ -581,6 +581,22 @@ static char *resource_string(char *buf, 
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)
+{
+	/* room for 6 * two hex digits, 5 colons and trailing zero */
+	char mac[18];
+	char *p = mac, *pend = mac + sizeof(mac);
+	int i;
+
+	for (i=0; i < 6; i++) {
+		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
+		*p++ = ':';
+	}
+	mac[17] = '\0';
+
+	return string(buf, end, mac, field_width, precision, flags);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -592,6 +608,8 @@ static char *resource_string(char *buf, 
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
@@ -607,6 +625,8 @@ static char *pointer(const char *fmt, ch
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {

Re: [PATCH] add %pM printf format specifier

[Joe Perches]

On Sat, 2008-10-25 at 01:46 +0200, Johannes Berg wrote:
> This adds a new printf format specifier for the kernel, %pM,
> to be used to print out MAC addresses. This has advantages
> over the current print_mac scheme:
>  * no need for DECLARE_MAC_BUF
>  * can be used safely in statements that might be compiled
>    out without the print_mac call staying.

Good things.

> --- everything.orig/lib/vsprintf.c	2008-10-25 01:21:54.000000000 +0200
> +++ everything/lib/vsprintf.c	2008-10-25 01:24:15.000000000 +0200
> @@ -581,6 +581,22 @@ static char *resource_string(char *buf, 
>  	return string(buf, end, sym, field_width, precision, flags);
>  }
>  
> +static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)
> +{
> +	/* room for 6 * two hex digits, 5 colons and trailing zero */
> +	char mac[18];
> +	char *p = mac, *pend = mac + sizeof(mac);
> +	int i;
> +
> +	for (i=0; i < 6; i++) {
> +		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
> +		*p++ = ':';
> +	}

For consistency, shouldn't the function name be mac_address_string?

I (and perhaps Harvey H) would be happier with:

	char mac[18];
	char *p = mac;
	int i;

	for (i = 0; i < 5; i++) {
		p = pack_hex_byte(p, addr[i]);
		*p++ = ':';
	}
	p = pack_hex_byte(p, addr[5]);
	*p = '\0';

Smaller, faster, etc...

Also, as the number of %p(foo) types increases, perhaps
something like sparse could validate the printf argument
types?

cheers, Joe

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Fri, 2008-10-24 at 18:41 -0700, Joe Perches wrote:


> > +static char *mac_address(char *buf, char *end, u8 *addr, int field_width, int precision, int flags)
> > +{
> > +	/* room for 6 * two hex digits, 5 colons and trailing zero */
> > +	char mac[18];
> > +	char *p = mac, *pend = mac + sizeof(mac);
> > +	int i;
> > +
> > +	for (i=0; i < 6; i++) {
> > +		p = number(p, pend, addr[i], 16, 2, -1, SMALL | ZEROPAD);
> > +		*p++ = ':';
> > +	}
> 
> For consistency, shouldn't the function name be mac_address_string?
> 
> I (and perhaps Harvey H) would be happier with:
> 
> 	char mac[18];
> 	char *p = mac;
> 	int i;
> 
> 	for (i = 0; i < 5; i++) {
> 		p = pack_hex_byte(p, addr[i]);
> 		*p++ = ':';
> 	}
> 	p = pack_hex_byte(p, addr[5]);
> 	*p = '\0';
> 
> Smaller, faster, etc...

Heh. I wasn't aware of pack_hex_byte. Not sure it really is smaller
though with the inlining, and kernel messages ought not to be
performance sensitive. OTOH, this is _way_ simpler in terms of code,
I'll send v2.

> Also, as the number of %p(foo) types increases, perhaps
> something like sparse could validate the printf argument
> types?

Hard to do really, what guarantees that a MAC address is 6 bytes, which
is the relevant thing? :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[Sven Anders]

[-- Attachment #1.1: Type: text/plain, Size: 910 bytes --]

Johannes Berg schrieb:
> This adds a new printf format specifier for the kernel, %pM,
> to be used to print out MAC addresses. This has advantages
> over the current print_mac scheme:
>  * no need for DECLARE_MAC_BUF
>  * can be used safely in statements that might be compiled
>    out without the print_mac call staying.

Nice, have you though of something similar for IPv4 and IPv6
adresses?

Regards
 Sven
-- 
 Sven Anders <anders@anduras.de>                 () Ascii Ribbon Campaign
                                                 /\ Support plain text e-mail
 ANDURAS service solutions AG
 Innstraße 71 - 94036 Passau - Germany
 Web: www.anduras.de - Tel: +49 (0)851-4 90 50-0 - Fax: +49 (0)851-4 90 50-55

Rechtsform: Aktiengesellschaft - Sitz: Passau - Amtsgericht Passau HRB 6032
Mitglieder des Vorstands: Sven Anders, Marcus Junker
Vorsitzender des Aufsichtsrats: Mark Peters

[-- Attachment #1.2: anders.vcf --]
[-- Type: text/x-vcard, Size: 352 bytes --]

begin:vcard
fn:Sven Anders
n:Anders;Sven
org:ANDURAS AG;Research and Development
adr;quoted-printable:;;Innstra=C3=9Fe 71;Passau;Bavaria;94036;Germany
email;internet:anders@anduras.de
title:Dipl. Inf.
tel;work:++49 (0)851 / 490 50 -0
tel;fax:++49 (0)851 / 590 50 - 55
x-mozilla-html:FALSE
url:http://www.anduras.de
version:2.1
end:vcard


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Sat, 2008-10-25 at 19:44 +0200, Sven Anders wrote:
> Johannes Berg schrieb:
> > This adds a new printf format specifier for the kernel, %pM,
> > to be used to print out MAC addresses. This has advantages
> > over the current print_mac scheme:
> >  * no need for DECLARE_MAC_BUF
> >  * can be used safely in statements that might be compiled
> >    out without the print_mac call staying.
> 
> Nice, have you though of something similar for IPv4 and IPv6
> adresses?

Not really, are they printed often? I haven't had to touch much code
that printed IP addresses. Nor have I seen them in logs a lot.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Sun, 26 Oct 2008 09:01:36 +0100

> On Sat, 2008-10-25 at 19:44 +0200, Sven Anders wrote:
> > Johannes Berg schrieb:
> > > This adds a new printf format specifier for the kernel, %pM,
> > > to be used to print out MAC addresses. This has advantages
> > > over the current print_mac scheme:
> > >  * no need for DECLARE_MAC_BUF
> > >  * can be used safely in statements that might be compiled
> > >    out without the print_mac call staying.
> > 
> > Nice, have you though of something similar for IPv4 and IPv6
> > adresses?
> 
> Not really, are they printed often? I haven't had to touch much code
> that printed IP addresses. Nor have I seen them in logs a lot.

It would be a nice future project for someone.  But don't worry
about it for now.

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

On Sun, 2008-10-26 at 01:03 -0700, David Miller wrote:

> > > Nice, have you though of something similar for IPv4 and IPv6
> > > adresses?
> > 
> > Not really, are they printed often? I haven't had to touch much code
> > that printed IP addresses. Nor have I seen them in logs a lot.
> 
> It would be a nice future project for someone.  But don't worry
> about it for now.

Of course, actually adding, say, %p4 and %p6 would be trivial, but
making lots of code use it so it's useful would be another mass
conversion I'm not too keen to do again right away :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

On Sun, 2008-10-26 at 09:21 +0100, Johannes Berg wrote:

> Of course, actually adding, say, %p4 and %p6 would be trivial, but
> making lots of code use it so it's useful would be another mass
> conversion I'm not too keen to do again right away :)

Actually... At least for ipv6 that isn't true, just do something like
this:

@@ include/linux/kernel.h
-#define NIP6(addr) \
-...
+#define NIP6(addr) &addr

-#define NIP6_FMT "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x"
-#define NIP6_SEQFMT "%04x%04x%04x%04x%04x%04x%04x%04x"
+#define NIP6_FMT "%p6"
+#define NIP6_SEQFMT "%#p6"

and then add the correct stuff to pointer() in lib/vsprintf.c, something
like

static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)
{
	char ip6[8*5];
	char *p = ip6;
	int i;
	u16 tmp;

	for (i=0; i<7; i++) {
		tmp = cpu_to_le16(addr[i]);
		p = pack_hex_byte(p, tmp >> 8);
		p = pack_hex_byte(p, tmp & 0xFF);
		if (!(flags & SPECIAL))
			*p++ = ':';
	}
	tmp = cpu_to_le16(addr[7]);
	p = pack_hex_byte(p, tmp >> 8);
	p = pack_hex_byte(p, tmp & 0xFF);
	*p = '\0';

	return string(buf, end, ip6, field_width, precision, flags & ~SPECIAL);
}


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[Harvey Harrison]

On Sun, 2008-10-26 at 09:30 +0100, Johannes Berg wrote:
> static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)
> {
> 	char ip6[8*5];
> 	char *p = ip6;
> 	int i;
> 	u16 tmp;
> 
> 	for (i=0; i<7; i++) {
> 		tmp = cpu_to_le16(addr[i]);

		tmp = be16_to_cpup(addr + i);

> 		p = pack_hex_byte(p, tmp >> 8);
> 		p = pack_hex_byte(p, tmp & 0xFF);
> 		if (!(flags & SPECIAL))
> 			*p++ = ':';
> 	}
> 	tmp = cpu_to_le16(addr[7]);

	tmp = be16_to_cpup(addr + 7);

Or maybe just cast addr to char * and avoid the byteswapping altogether.

Just a thought,

Harvey

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On Sun, 2008-10-26 at 12:01 -0700, Harvey Harrison wrote:
> On Sun, 2008-10-26 at 09:30 +0100, Johannes Berg wrote:
> > static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)
> > {
> > 	char ip6[8*5];
> > 	char *p = ip6;
> > 	int i;
> > 	u16 tmp;
> > 
> > 	for (i=0; i<7; i++) {
> > 		tmp = cpu_to_le16(addr[i]);
> 
> 		tmp = be16_to_cpup(addr + i);
> 
> > 		p = pack_hex_byte(p, tmp >> 8);
> > 		p = pack_hex_byte(p, tmp & 0xFF);
> > 		if (!(flags & SPECIAL))
> > 			*p++ = ':';
> > 	}
> > 	tmp = cpu_to_le16(addr[7]);
> 
> 	tmp = be16_to_cpup(addr + 7);
> 
> Or maybe just cast addr to char * and avoid the byteswapping altogether.

Yeah, this was obviously not tested. Want to build it? I don't have
access to a fast build machine right now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[Harvey Harrison]

On Sun, 2008-10-26 at 20:33 +0100, Johannes Berg wrote:
> On Sun, 2008-10-26 at 12:01 -0700, Harvey Harrison wrote:
> > On Sun, 2008-10-26 at 09:30 +0100, Johannes Berg wrote:
> > > static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)
> > > {
> > > 	char ip6[8*5];
> > > 	char *p = ip6;
> > > 	int i;
> > > 	u16 tmp;
> > > 
> > > 	for (i=0; i<7; i++) {
> > > 		tmp = cpu_to_le16(addr[i]);
> > 
> > 		tmp = be16_to_cpup(addr + i);
> > 
> > > 		p = pack_hex_byte(p, tmp >> 8);
> > > 		p = pack_hex_byte(p, tmp & 0xFF);
> > > 		if (!(flags & SPECIAL))
> > > 			*p++ = ':';
> > > 	}
> > > 	tmp = cpu_to_le16(addr[7]);
> > 
> > 	tmp = be16_to_cpup(addr + 7);
> > 
> > Or maybe just cast addr to char * and avoid the byteswapping altogether.
> 
> Yeah, this was obviously not tested. Want to build it? I don't have
> access to a fast build machine right now.
> 

Could you send me whatever you've got currently and I'll have a go at
it.

Cheers,

Harvey

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

On Sun, 2008-10-26 at 12:38 -0700, Harvey Harrison wrote:
> On Sun, 2008-10-26 at 20:33 +0100, Johannes Berg wrote:
> > On Sun, 2008-10-26 at 12:01 -0700, Harvey Harrison wrote:
> > > On Sun, 2008-10-26 at 09:30 +0100, Johannes Berg wrote:
> > > > static char *ip6_string(char *buf, char *end, be16 *addr, int field_width, int precision, int flags)
> > > > {
> > > > 	char ip6[8*5];
> > > > 	char *p = ip6;
> > > > 	int i;
> > > > 	u16 tmp;
> > > > 
> > > > 	for (i=0; i<7; i++) {
> > > > 		tmp = cpu_to_le16(addr[i]);
> > > 
> > > 		tmp = be16_to_cpup(addr + i);
> > > 
> > > > 		p = pack_hex_byte(p, tmp >> 8);
> > > > 		p = pack_hex_byte(p, tmp & 0xFF);
> > > > 		if (!(flags & SPECIAL))
> > > > 			*p++ = ':';
> > > > 	}
> > > > 	tmp = cpu_to_le16(addr[7]);
> > > 
> > > 	tmp = be16_to_cpup(addr + 7);
> > > 
> > > Or maybe just cast addr to char * and avoid the byteswapping altogether.
> > 
> > Yeah, this was obviously not tested. Want to build it? I don't have
> > access to a fast build machine right now.
> > 
> 
> Could you send me whatever you've got currently and I'll have a go at
> it.

I don't have anything. I typed this into the email directly.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[RFC PATCH] printk: add the %pM, %p4, %p6 format specifiers

[Harvey Harrison]

Add format specifiers for printing out colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

IPv4 addresses (%p4):
xx:xx:xx:xx

IPv6 addresses (%p6):
xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx

%#pM, %#p4, %#p6 are also supported and print without the colon
separators.

[Based on Johannes Berg's initial patch]
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 lib/vsprintf.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a013bbc..eb19471 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -581,6 +581,58 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *ip4_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip4_addr[4 * 3]; /* (4 * 2 hex digits), 3 colons and trailing zero */
+	char *p = ip4_addr;
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 3)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip4_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *ip6_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */
+	char *p = ip6_addr;
+	int i;
+
+	for (i = 0; i < 7; i++) {
+		p = pack_hex_byte(p, addr[2 * i]);
+		p = pack_hex_byte(p, addr[2 * i + 1]);
+		if (!(flags & SPECIAL) && i != 6)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip6_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 5; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 4)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
+ *       big-endian 16 bit hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
@@ -607,6 +665,12 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
+	case '4':
+		return ip4_address(buf, end, ptr, field_width, precision, flags);
+	case '6':
+		return ip6_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
-- 
1.6.0.3.729.g6ea410

[RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[Harvey Harrison]

Add format specifiers for printing out colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

IPv4 addresses (%p4):
xx:xx:xx:xx

IPv6 addresses (%p6):
xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx

%#pM, %#p4, %#p6 are also supported and print without the colon
separators.

[Based on Johannes Berg's initial patch]
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
This one without the embarrassing index typos in ip6_address and mac_address.

 lib/vsprintf.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a013bbc..eec3879 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -581,6 +581,58 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *ip4_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip4_addr[4 * 3]; /* (4 * 2 hex digits), 3 colons and trailing zero */
+	char *p = ip4_addr;
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 3)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip4_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *ip6_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */
+	char *p = ip6_addr;
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		p = pack_hex_byte(p, addr[2 * i]);
+		p = pack_hex_byte(p, addr[2 * i + 1]);
+		if (!(flags & SPECIAL) && i != 7)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, ip6_addr, field_width, precision, flags & ~SPECIAL);
+}
+
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 6; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 5)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - '4' For a 4-byte IPv4 address, it prints the address in the
+ *       usual colon-separated hex notation
+ * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
+ *       big-endian 16 bit hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
@@ -607,6 +665,12 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
+	case '4':
+		return ip4_address(buf, end, ptr, field_width, precision, flags);
+	case '6':
+		return ip6_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
-- 
1.6.0.3.729.g6ea410

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[Frans Pop]

Harvey Harrison wrote:
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index a013bbc..eec3879 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -592,6 +644,12 @@ static char *resource_string(char *buf, char *end,
> struct resource *res, int fie
>   * - 'S' For symbolic direct pointers
>   * - 'R' For a struct resource pointer, it prints the range of
>   *       addresses (not the name nor the flags)
> + * - 'M' For a 6-byte MAC address, it prints the address in the
> + *       usual colon-separated hex notation
> + * - '4' For a 4-byte IPv4 address, it prints the address in the
> + *       usual colon-separated hex notation
> + * - 'M' For a 16-byte IPv6 address, it prints the address in colon separated
> + *       big-endian 16 bit hex notation

Shouldn't that last one be '6' instead of 'M'?

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[H. Peter Anvin]

Frans Pop wrote:
>> + * - '4' For a 4-byte IPv4 address, it prints the address in the
>> + *       usual colon-separated hex notation

Since when are IPv4 addresses "usually" printed in colon-separated hex?

	-hpa

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

On Sun, 2008-10-26 at 17:31 -0700, Harvey Harrison wrote:
> Add format specifiers for printing out colon-separated bytes:
> 
> MAC addresses (%pM):
> xx:xx:xx:xx:xx:xx
> 
> IPv4 addresses (%p4):
> xx:xx:xx:xx

As hpa points out, the usual notation would be %d.%d.%d.%d :)

> %#pM, %#p4, %#p6 are also supported and print without the colon
> separators.

The # for ipv4 we might use for cpu-endian rather than network-endian
maybe?

Also can you keep mac/ip address patches split up so we can use my
version of the mac address patch (it's already well-tested)?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[Harvey Harrison]

On Mon, 2008-10-27 at 07:59 +0100, Johannes Berg wrote:

> Also can you keep mac/ip address patches split up so we can use my
> version of the mac address patch (it's already well-tested)?
> 

Sure.  I'll build on top.

Harvey

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[David Miller]

From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Mon, 27 Oct 2008 09:28:24 -0700

> On Mon, 2008-10-27 at 07:59 +0100, Johannes Berg wrote:
> 
> > Also can you keep mac/ip address patches split up so we can use my
> > version of the mac address patch (it's already well-tested)?
> > 
> 
> Sure.  I'll build on top.

So I'll wait for Harvey's "v3" of this patch, and once I have that in
the net-next-2.6 tree I'll start adding Johannes's conversions on top.

Re: [RFC PATCHv2] printk: add the %pM, %p4, %p6 format specifiers

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

On Mon, 2008-10-27 at 12:38 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Mon, 27 Oct 2008 09:28:24 -0700
> 
> > On Mon, 2008-10-27 at 07:59 +0100, Johannes Berg wrote:
> > 
> > > Also can you keep mac/ip address patches split up so we can use my
> > > version of the mac address patch (it's already well-tested)?
> > > 
> > 
> > Sure.  I'll build on top.
> 
> So I'll wait for Harvey's "v3" of this patch, and once I have that in
> the net-next-2.6 tree I'll start adding Johannes's conversions on top.

Ok, fine with me too. Harvey, can you sort out the ip4 issue and then
send with %pM included?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Harvey Harrison]

Add format specifiers for printing out six colon-separated bytes:

MAC addresses (%pM):
xx:xx:xx:xx:xx:xx

%#pM is also supported and omits the colon separators.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Dave, this passes testing here, but I was wondering if perhaps it would be
better to allow a length to be specified as well, which would allow:

%pM6 for mac addresses, etc as there seem a lot of places in kernel that
print out a list of colon separated bytes of various lengths.

But if that was added, it may be more natural to call it 
%pB (bytes)
%pW (words)

Then mac addresses would be %pB6
IPv6 addresses would be %pW8 (8 words)

It would be trivial to add, but maybe I'm overthinking this.  In any event,
this patch only adds %pM for mac addresses.

 lib/vsprintf.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a013bbc..2025305 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -581,6 +581,23 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
 	return string(buf, end, sym, field_width, precision, flags);
 }
 
+static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
+			 int precision, int flags)
+{
+	char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */
+	char *p = mac_addr;
+	int i;
+
+	for (i = 0; i < 6; i++) {
+		p = pack_hex_byte(p, addr[i]);
+		if (!(flags & SPECIAL) && i != 5)
+			*p++ = ':';
+	}
+	*p = '\0';
+
+	return string(buf, end, mac_addr, field_width, precision, flags & ~SPECIAL);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -592,6 +609,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
  * - 'S' For symbolic direct pointers
  * - 'R' For a struct resource pointer, it prints the range of
  *       addresses (not the name nor the flags)
+ * - 'M' For a 6-byte MAC address, it prints the address in the
+ *       usual colon-separated hex notation
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
@@ -607,6 +626,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
 	case 'R':
 		return resource_string(buf, end, ptr, field_width, precision, flags);
+	case 'M':
+		return mac_address(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {
-- 
1.6.0.3.729.g6ea410

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Joe Perches]

On Mon, 2008-10-27 at 12:59 -0700, Harvey Harrison wrote:

The changes to use %p4 aren't too bad.
I did that last year using a similar style
to print_mac.  80 or so files.
http://repo.or.cz/w/linux-2.6/trivial-mods.git?a=shortlog;h=refs/heads/print_ipv4
ipv6 was 30 or so files
http://repo.or.cz/w/linux-2.6/trivial-mods.git?a=shortlog;h=refs/heads/print_ipv6

> I was wondering if perhaps it would be
> better to allow a length to be specified as well, which would allow:
[]
> But if that was added, it may be more natural to call it 
> %pB (bytes)
> %pW (words)

I think length would not be good addition.
print_dump_hex already does a fine job.
There are also BE/LE expectation problems with pW.

The %p6 pointer should be in6_addr *
The %p4 pointer should be __be32 *.
Printing an ipv4 address should use %u.%u.%u.%u

I've been doodling with sparse to check
calls with __attribute__(format(printf
parsing the format strings for %p<FOO>
and match the argument types.

I'll post it if anything comes of it.

> +static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
> +			 int precision, int flags)
> +{

mac_address_string please.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Mon, 27 Oct 2008 14:55:15 -0700

> On Mon, 2008-10-27 at 12:59 -0700, Harvey Harrison wrote:
> 
> > +static char *mac_address(char *buf, char *end, u8 *addr, int field_width,
> > +			 int precision, int flags)
> > +{
> 
> mac_address_string please.

I'll make this change when I apply Harvey's patch.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[David Miller]

From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Mon, 27 Oct 2008 12:59:02 -0700

> Add format specifiers for printing out six colon-separated bytes:
> 
> MAC addresses (%pM):
> xx:xx:xx:xx:xx:xx
> 
> %#pM is also supported and omits the colon separators.
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>

Applied, thanks.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Harvey Harrison]

On Mon, 2008-10-27 at 15:47 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Mon, 27 Oct 2008 12:59:02 -0700
> 
> > Add format specifiers for printing out six colon-separated bytes:
> > 
> > MAC addresses (%pM):
> > xx:xx:xx:xx:xx:xx
> > 
> > %#pM is also supported and omits the colon separators.
> > 
> > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> 
> Applied, thanks.

Did you happen to have a preference with regard to the specifier for
IPv6 addresses:

I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
%#pI6 for NIP6_SEQFMT.

On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
the periods isn't useful?

What do you think of that?

Harvey

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Alexey Dobriyan]

On Mon, Oct 27, 2008 at 04:14:14PM -0700, Harvey Harrison wrote:
> On Mon, 2008-10-27 at 15:47 -0700, David Miller wrote:
> > From: Harvey Harrison <harvey.harrison@gmail.com>
> > Date: Mon, 27 Oct 2008 12:59:02 -0700
> > 
> > > Add format specifiers for printing out six colon-separated bytes:
> > > 
> > > MAC addresses (%pM):
> > > xx:xx:xx:xx:xx:xx
> > > 
> > > %#pM is also supported and omits the colon separators.
> > > 
> > > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > 
> > Applied, thanks.
> 
> Did you happen to have a preference with regard to the specifier for
> IPv6 addresses:
> 
> I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
> %#pI6 for NIP6_SEQFMT.
> 
> On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
> and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
> the periods isn't useful?

%#pI4 is horrible and %p4 and %p6 were cool.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Joe Perches]

On Mon, 2008-10-27 at 16:14 -0700, Harvey Harrison wrote:
> I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
> %#pI6 for NIP6_SEQFMT.
> 
> On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
> and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
> the periods isn't useful?

HIPQUAD is horrible and should disappear.
Just use htonl first.

I think just %p4 %p6 %#p6 for the NIP6_SEQFMT uses.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

On Mon, 2008-10-27 at 16:48 -0700, Joe Perches wrote:
> On Mon, 2008-10-27 at 16:14 -0700, Harvey Harrison wrote:
> > I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
> > %#pI6 for NIP6_SEQFMT.
> > 
> > On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
> > and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
> > the periods isn't useful?
> 
> HIPQUAD is horrible and should disappear.
> Just use htonl first.

But you can't really say

printk("... %p4 ...", &htonl(host_ip4));

can you? I suppose the compiler would actually create a variable for
that?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[David Miller]

From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Mon, 27 Oct 2008 16:14:14 -0700

> Did you happen to have a preference with regard to the specifier for
> IPv6 addresses:
> 
> I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
> %#pI6 for NIP6_SEQFMT.
> 
> On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
> and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
> the periods isn't useful?
> 
> What do you think of that?

I'm ok with the '#' modifier but remind me again what's wrong with
using plain %p4 and %p6?

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Harvey Harrison]

On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Mon, 27 Oct 2008 16:14:14 -0700
> 
> > Did you happen to have a preference with regard to the specifier for
> > IPv6 addresses:
> > 
> > I was thinking of using %pI6 to replace NIP6() and NIP6_FMT and using
> > %#pI6 for NIP6_SEQFMT.
> > 
> > On the IPv4 side, maybe use %pI4 for network endian NIPQUAD() and NIPQUAD_FMT
> > and then %#pI4 for host-endian HIQUAD(), as displaying the IPv4 address without
> > the periods isn't useful?
> > 
> > What do you think of that?
> 
> I'm ok with the '#' modifier but remind me again what's wrong with
> using plain %p4 and %p6?

Nothing at all, just wanted to know what color bikeshed you wanted.

I'll go with %p4 %p6.

Harvey

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Joe Perches]

On Mon, 2008-10-27 at 17:05 -0700, Harvey Harrison wrote:
> On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> > I'm ok with the '#' modifier but remind me again what's wrong with
> > using plain %p4 and %p6?
> Nothing at all, just wanted to know what color bikeshed you wanted.
> I'll go with %p4 %p6.

Just an FYI for v6 addresses:

There are a few uses of ipv6 address as "%08X%08X%08X%08X" 
Pavel Emelyanov tried to isolate them with a suggested patch:

http://www.mail-archive.com/netdev@vger.kernel.org/msg51655.html

These addresses are not shown in network order, but are
printed backwards in host order. Oops.  Apparently it's
too late to fix them.

Re: [RFC PATCHv3] printk: add %pM format specifier for MAC addresses

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On Mon, 2008-10-27 at 17:22 -0700, Joe Perches wrote:
> On Mon, 2008-10-27 at 17:05 -0700, Harvey Harrison wrote:
> > On Mon, 2008-10-27 at 16:55 -0700, David Miller wrote:
> > > I'm ok with the '#' modifier but remind me again what's wrong with
> > > using plain %p4 and %p6?
> > Nothing at all, just wanted to know what color bikeshed you wanted.
> > I'll go with %p4 %p6.
> 
> Just an FYI for v6 addresses:
> 
> There are a few uses of ipv6 address as "%08X%08X%08X%08X" 
> Pavel Emelyanov tried to isolate them with a suggested patch:
> 
> http://www.mail-archive.com/netdev@vger.kernel.org/msg51655.html
> 
> These addresses are not shown in network order, but are
> printed backwards in host order. Oops.  Apparently it's
> too late to fix them.

So that code we don't touch here at all.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] add %pM printf format specifier

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On Sun, 2008-10-26 at 09:21 +0100, Johannes Berg wrote:

> > It would be a nice future project for someone.  But don't worry
> > about it for now.
> 
> Of course, actually adding, say, %p4 and %p6 would be trivial, but
> making lots of code use it so it's useful would be another mass
> conversion I'm not too keen to do again right away :)

And for IPv4 you can do it in a similar way, except for HIPQUAD which
you'd have to actually replace in the code, but there are less than 20
instances of it. Maybe use SPECIAL to indicate host byte order or
something, I'll leave that to the interested, unless I get really bored
next week (I'm kinda bored right now but don't have access to a fast
build machine ;) )

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]