| Message ID | 1341310000-4082-2-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2012-07-03 at 13:06 +0300, Andy Shevchenko wrote: > There are many places in the kernel where the drivers print small buffers as a > hex string. This patch adds a support of the variable width buffer to print it > as a hex string with a delimiter. The idea came from Pavel Roskin here: > http://www.digipedia.pl/usenet/thread/18835/17449/ > > Sample output of > pr_info("buf[%d:%d] %*pM\n", from, len, len, &buf[from]); > could be look like this: > [ 0.726130] buf[51:8] e8:16:b6:ef:e3:74:45:6e > [ 0.750736] buf[59:15] 31:81:b8:3f:35:49:06:ae:df:32:06:05:4a:af:55 > [ 0.757602] buf[17:5] ac:16:d5:2c:ef Hi Andy. > diff --git a/lib/vsprintf.c b/lib/vsprintf.c [] > @@ -655,12 +655,13 @@ char *resource_string(char *buf, char *end, struct resource *res, [] > +char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, > + const char *fmt) [] > @@ -678,18 +679,31 @@ char *mac_address_string(char *buf, char *end, u8 *addr, [] > + while (i < len) { Oh good, a while loop, thanks. [] > @@ -947,6 +961,9 @@ int kptr_restrict __read_mostly; > * - 'MF' For a 6-byte MAC FDDI address, it prints the address > * with a dash-separated hex notation > * - '[mM]R For a 6-byte MAC address, Reverse order (Bluetooth) > + * Optional usage is %*p[Mn][FR] with variable length to print. It > + * supports up to 64 bytes of the input. Consider to use print_hex_dump() > + * for the larger input. It might be more sensible to use new, distinct "%*pH" and "%*ph" functions and not touch the mac address function at all. Will anyone ever really want to emit the buffer in reverse? I don't think so. Perhaps when using a hex_string_buffer func the separator should be a space/no-space with %*pHh. You could extend the max to 128 or larger now. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 3, 2012 at 6:33 PM, Joe Perches <joe@perches.com> wrote: > On Tue, 2012-07-03 at 13:06 +0300, Andy Shevchenko wrote: >> There are many places in the kernel where the drivers print small buffers as a >> hex string. This patch adds a support of the variable width buffer to print it >> as a hex string with a delimiter. The idea came from Pavel Roskin here: >> http://www.digipedia.pl/usenet/thread/18835/17449/ >> >> Sample output of >> pr_info("buf[%d:%d] %*pM\n", from, len, len, &buf[from]); >> could be look like this: >> [ 0.726130] buf[51:8] e8:16:b6:ef:e3:74:45:6e >> [ 0.750736] buf[59:15] 31:81:b8:3f:35:49:06:ae:df:32:06:05:4a:af:55 >> [ 0.757602] buf[17:5] ac:16:d5:2c:ef > It might be more sensible to use new, distinct > "%*pH" and "%*ph" functions and not touch the > mac address function at all. Will anyone ever > really want to emit the buffer in reverse? > I don't think so. Yeah, probably it's only the case for the Bluetooth addresses. > Perhaps when using a hex_string_buffer func the > separator should be a space/no-space with %*pHh. What I learned from today's linux-next is the most used separators are ' ' (space), '' (nothing), ':' and '-'. We have dozens of the cases for first three. The '-' support could not be implemented nevertheless. So, might be %*pHh[CDS] C for 'colon', S for 'space', D for 'dash' looks better. 'Hh' for capital/small letters than? > You could extend the max to 128 or larger now. I don't think it is really needed. Most of the current cases usually print not more than ~30bytes (in average) per time. And I couldn't imagine good looking printing for long lines anyway.
On Tue, 2012-07-03 at 21:32 +0300, Andy Shevchenko wrote: > On Tue, Jul 3, 2012 at 6:33 PM, Joe Perches <joe@perches.com> wrote: > > On Tue, 2012-07-03 at 13:06 +0300, Andy Shevchenko wrote: > >> There are many places in the kernel where the drivers print small buffers as a > >> hex string. This patch adds a support of the variable width buffer to print it > >> as a hex string with a delimiter. The idea came from Pavel Roskin here: > >> http://www.digipedia.pl/usenet/thread/18835/17449/ > >> > >> Sample output of > >> pr_info("buf[%d:%d] %*pM\n", from, len, len, &buf[from]); > >> could be look like this: > >> [ 0.726130] buf[51:8] e8:16:b6:ef:e3:74:45:6e > >> [ 0.750736] buf[59:15] 31:81:b8:3f:35:49:06:ae:df:32:06:05:4a:af:55 > >> [ 0.757602] buf[17:5] ac:16:d5:2c:ef > > > It might be more sensible to use new, distinct > > "%*pH" and "%*ph" functions and not touch the > > mac address function at all. Will anyone ever > > really want to emit the buffer in reverse? > > I don't think so. > Yeah, probably it's only the case for the Bluetooth addresses. > > > Perhaps when using a hex_string_buffer func the > > separator should be a space/no-space with %*pHh. > What I learned from today's linux-next is the most used separators are > ' ' (space), '' (nothing), ':' and '-'. We have dozens of the cases > for first three. The '-' support could not be implemented > nevertheless. > So, might be %*pHh[CDS] C for 'colon', S for 'space', D for 'dash' looks better. Maybe use a space default. > 'Hh' for capital/small letters than? If you want, though I'd hope nobody uses upper case. > > You could extend the max to 128 or larger now. > I don't think it is really needed. I hope it's not, but I just don't see the need to limit it. > Most of the current cases usually > print not more than ~30bytes (in average) per time. And I couldn't > imagine good looking printing for long lines anyway. Yup, they'd be ugly. print_hex_dump() should be favored anyway. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jul 03, 2012 at 11:48:00AM -0700, Joe Perches wrote: > On Tue, 2012-07-03 at 21:32 +0300, Andy Shevchenko wrote: > > On Tue, Jul 3, 2012 at 6:33 PM, Joe Perches <joe@perches.com> wrote: > > > On Tue, 2012-07-03 at 13:06 +0300, Andy Shevchenko wrote: > > >> There are many places in the kernel where the drivers print small buffers as a > > >> hex string. This patch adds a support of the variable width buffer to print it > > >> as a hex string with a delimiter. The idea came from Pavel Roskin here: > > >> http://www.digipedia.pl/usenet/thread/18835/17449/ > > >> > > >> Sample output of > > >> pr_info("buf[%d:%d] %*pM\n", from, len, len, &buf[from]); > > >> could be look like this: > > >> [ 0.726130] buf[51:8] e8:16:b6:ef:e3:74:45:6e > > >> [ 0.750736] buf[59:15] 31:81:b8:3f:35:49:06:ae:df:32:06:05:4a:af:55 > > >> [ 0.757602] buf[17:5] ac:16:d5:2c:ef The idea is really good and can make redundant lots of print_hex functions. > > > > > It might be more sensible to use new, distinct > > > "%*pH" and "%*ph" functions and not touch the > > > mac address function at all. Will anyone ever > > > really want to emit the buffer in reverse? > > > I don't think so. > > Yeah, probably it's only the case for the Bluetooth addresses. > > > > > Perhaps when using a hex_string_buffer func the > > > separator should be a space/no-space with %*pHh. > > What I learned from today's linux-next is the most used separators are > > ' ' (space), '' (nothing), ':' and '-'. We have dozens of the cases > > for first three. The '-' support could not be implemented > > nevertheless. > > So, might be %*pHh[CDS] C for 'colon', S for 'space', D for 'dash' looks better. > > Maybe use a space default. I do not think we need other specifier then space at all. > > > 'Hh' for capital/small letters than? > > If you want, though I'd hope nobody uses upper case. > > > > You could extend the max to 128 or larger now. > > I don't think it is really needed. > > I hope it's not, but I just don't see the need to limit it. > > > Most of the current cases usually > > print not more than ~30bytes (in average) per time. And I couldn't > > imagine good looking printing for long lines anyway. Maybe insert '\n' after 16 numbers? > > Yup, they'd be ugly. > print_hex_dump() should be favored anyway. For print_hex_dump we would need to invent unreadable constructions with defines like you mentioned in other thread: #ifdef SOME_BLUETOOTH_DEBUG_FLAG #define bt_hex_dump_dbg(...) \ print_hex_dump(...) #else #define bt_hex_dump_dbg(...) \ do { } while (0) #endif Best regards Andrei Emeltchenko --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index 90ff4d7..3ae3d32 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -73,6 +73,11 @@ MAC/FDDI addresses: specifier to use reversed byte order suitable for visual interpretation of Bluetooth addresses which are in the little endian order. + Optional usage of all of the above is to specify variable length via + putting '*' into the specificator ('%*p[Mm][FR]'). In this case it will + print up to 64 bytes of the input as a hex string with certain + separator. For larger buffers consider to use print_hex_dump(). + IPv4 addresses: %pI4 1.2.3.4 diff --git a/lib/vsprintf.c b/lib/vsprintf.c index c65f5d4..ef4bbd2 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -655,12 +655,13 @@ char *resource_string(char *buf, char *end, struct resource *res, } static noinline_for_stack -char *mac_address_string(char *buf, char *end, u8 *addr, - struct printf_spec spec, const char *fmt) +char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, + const char *fmt) { - char mac_addr[sizeof("xx:xx:xx:xx:xx:xx")]; - char *p = mac_addr; - int i; + char hex_str[8*3+1]; /* support up to 8 bytes to print */ + int len = 6; /* default length is 6 bytes */ + char *p; + int i = 0, j; char separator; bool reversed = false; @@ -678,18 +679,31 @@ char *mac_address_string(char *buf, char *end, u8 *addr, break; } - for (i = 0; i < 6; i++) { - if (reversed) - p = hex_byte_pack(p, addr[5 - i]); - else - p = hex_byte_pack(p, addr[i]); + if (spec.field_width > 0) + len = min_t(int, spec.field_width, 64); + + while (i < len) { + p = hex_str; + for (j = 0; j < 8 && i < len; j++) { + if (reversed) + p = hex_byte_pack(p, addr[len - 1 - i]); + else + p = hex_byte_pack(p, addr[i]); + + if (fmt[0] == 'M' && i != len - 1) + *p++ = separator; + i++; + } + *p = '\0'; - if (fmt[0] == 'M' && i != 5) - *p++ = separator; + for (p = hex_str; *p != '\0'; p++) { + if (buf < end) + *buf = *p; + ++buf; + } } - *p = '\0'; - return string(buf, end, mac_addr, spec); + return buf; } static noinline_for_stack @@ -947,6 +961,9 @@ int kptr_restrict __read_mostly; * - 'MF' For a 6-byte MAC FDDI address, it prints the address * with a dash-separated hex notation * - '[mM]R For a 6-byte MAC address, Reverse order (Bluetooth) + * Optional usage is %*p[Mn][FR] with variable length to print. It + * supports up to 64 bytes of the input. Consider to use print_hex_dump() + * for the larger input. * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way * IPv4 uses dot-separated decimal without leading 0's (1.2.3.4) * IPv6 uses colon separated network-order 16 bit hex with leading 0's @@ -1011,7 +1028,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'm': /* Contiguous: 000102030405 */ /* [mM]F (FDDI) */ /* [mM]R (Reverse order; Bluetooth) */ - return mac_address_string(buf, end, ptr, spec, fmt); + return hex_string(buf, end, ptr, spec, fmt); case 'I': /* Formatted IP supported * 4: 1.2.3.4 * 6: 0001:0203:...:0708 @@ -1291,6 +1308,8 @@ qualifier: * %pMF output a 6-byte MAC address with dashes * %pm output a 6-byte MAC address without colons * %pmR output a 6-byte MAC address without colons in reversed order + * %*p[Mm][FR] a variable-length hex string with a separator (supports up to 64 + * bytes of the input) * %pI4 print an IPv4 address without leading zeros * %pi4 print an IPv4 address with leading zeros * %pI6 print an IPv6 address with colons
There are many places in the kernel where the drivers print small buffers as a hex string. This patch adds a support of the variable width buffer to print it as a hex string with a delimiter. The idea came from Pavel Roskin here: http://www.digipedia.pl/usenet/thread/18835/17449/ Sample output of pr_info("buf[%d:%d] %*pM\n", from, len, len, &buf[from]); could be look like this: [ 0.726130] buf[51:8] e8:16:b6:ef:e3:74:45:6e [ 0.750736] buf[59:15] 31:81:b8:3f:35:49:06:ae:df:32:06:05:4a:af:55 [ 0.757602] buf[17:5] ac:16:d5:2c:ef Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- Documentation/printk-formats.txt | 5 ++++ lib/vsprintf.c | 49 ++++++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 15 deletions(-)