| Message ID | 20190508070148.23130-4-alastair@au1.ibm.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | Hexdump Enhancements | expand |
On 5/8/19 12:01 AM, Alastair D'Silva wrote: > From: Alastair D'Silva <alastair@d-silva.org> > > Some buffers may only be partially filled with useful data, while the rest > is padded (typically with 0x00 or 0xff). > > This patch introduces a flag to allow the supression of lines of repeated > bytes, which are replaced with '** Skipped %u bytes of value 0x%x **' > > An inline wrapper function is provided for backwards compatibility with > existing code, which maintains the original behaviour. > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > --- > include/linux/printk.h | 25 +++++++++--- > lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++------ > 2 files changed, 99 insertions(+), 17 deletions(-) > Hi, Did you do "make htmldocs" or something similar on this? > diff --git a/lib/hexdump.c b/lib/hexdump.c > index 3943507bc0e9..d61a1e4f19fa 100644 > --- a/lib/hexdump.c > +++ b/lib/hexdump.c > @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > EXPORT_SYMBOL(hex_dump_to_buffer); > > #ifdef CONFIG_PRINTK > + > +/** > + * Check if a buffer contains only a single byte value > + * @buf: pointer to the buffer > + * @len: the size of the buffer in bytes > + * @val: outputs the value if if the bytes are identical Does this work without a function name? Documentation/doc-guide/kernel-doc.rst says the general format is: /** * function_name() - Brief description of function. * @arg1: Describe the first argument. * @arg2: Describe the second argument. * One can provide multiple line descriptions * for arguments. * > + */ > /** > - * print_hex_dump - print a text hex dump to syslog for a binary blob of data > + * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal Also not in the general documented format. > * @level: kernel log level (e.g. KERN_DEBUG) > * @prefix_str: string to prefix each line with; > * caller supplies trailing spaces for alignment if desired
On Wed, 2019-05-08 at 17:58 -0700, Randy Dunlap wrote: > On 5/8/19 12:01 AM, Alastair D'Silva wrote: > > From: Alastair D'Silva <alastair@d-silva.org> > > > > Some buffers may only be partially filled with useful data, while > > the rest > > is padded (typically with 0x00 or 0xff). > > > > This patch introduces a flag to allow the supression of lines of > > repeated > > bytes, which are replaced with '** Skipped %u bytes of value 0x%x > > **' > > > > An inline wrapper function is provided for backwards compatibility > > with > > existing code, which maintains the original behaviour. > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org> > > --- > > include/linux/printk.h | 25 +++++++++--- > > lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++ > > ------ > > 2 files changed, 99 insertions(+), 17 deletions(-) > > > > Hi, > Did you do "make htmldocs" or something similar on this? > > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > index 3943507bc0e9..d61a1e4f19fa 100644 > > --- a/lib/hexdump.c > > +++ b/lib/hexdump.c > > @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t > > len, int rowsize, int groupsize, > > EXPORT_SYMBOL(hex_dump_to_buffer); > > > > #ifdef CONFIG_PRINTK > > + > > +/** > > + * Check if a buffer contains only a single byte value > > + * @buf: pointer to the buffer > > + * @len: the size of the buffer in bytes > > + * @val: outputs the value if if the bytes are identical > > Does this work without a function name? > Documentation/doc-guide/kernel-doc.rst says the general format is: > > /** > * function_name() - Brief description of function. > * @arg1: Describe the first argument. > * @arg2: Describe the second argument. > * One can provide multiple line descriptions > * for arguments. > * > > > + */ > > /** > > - * print_hex_dump - print a text hex dump to syslog for a binary > > blob of data > > + * print_hex_dump_ext: dump a binary blob of data to syslog in > > hexadecimal > > Also not in the general documented format. > Thanks Randy, I'll address these.
Hi Alastair, Thanks for your patch! On Wed, May 8, 2019 at 9:04 AM Alastair D'Silva <alastair@au1.ibm.com> wrote: > From: Alastair D'Silva <alastair@d-silva.org> > > Some buffers may only be partially filled with useful data, while the rest > is padded (typically with 0x00 or 0xff). > > This patch introduces a flag to allow the supression of lines of repeated > bytes, Given print_hex_dump() operates on entities of groupsize (1, 2, 4, or 8) bytes, wouldn't it make more sense to consider repeated groups instead of repeated bytes? > which are replaced with '** Skipped %u bytes of value 0x%x **' Using a custom message instead of just "*", like "hexdump" uses, will require preprocessing the output when recovering the original binary data by feeding it to e.g. "xxd". This may sound worse than it is, though, as I never got "xxd" to work without preprocessing anyway ;-) $ cat $(type -p unhexdump) #!/bin/sh sed 's/^[0-9a-f]*//' $1 | xxd -r -p | dd conv=swab Gr{oetje,eeting}s, Geert
> -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Monday, 13 May 2019 5:01 PM > To: Alastair D'Silva <alastair@au1.ibm.com> > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>; Joonas > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter > <daniel@ffwll.ch>; Dan Carpenter <dan.carpenter@oracle.com>; Karsten > Keil <isdn@linux-pingi.de>; Jassi Brar <jassisinghbrar@gmail.com>; Tom > Lendacky <thomas.lendacky@amd.com>; David S. Miller > <davem@davemloft.net>; Jose Abreu <Jose.Abreu@synopsys.com>; Kalle > Valo <kvalo@codeaurora.org>; Stanislaw Gruszka <sgruszka@redhat.com>; > Benson Leung <bleung@chromium.org>; Enric Balletbo i Serra > <enric.balletbo@collabora.com>; James E.J. Bottomley > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro > <viro@zeniv.linux.org.uk>; Petr Mladek <pmladek@suse.com>; Sergey > Senozhatsky <sergey.senozhatsky@gmail.com>; Steven Rostedt > <rostedt@goodmis.org>; David Laight <David.Laight@aculab.com>; Andrew > Morton <akpm@linux-foundation.org>; Intel Graphics Development <intel- > gfx@lists.freedesktop.org>; DRI Development <dri- > devel@lists.freedesktop.org>; Linux Kernel Mailing List <linux- > kernel@vger.kernel.org>; netdev <netdev@vger.kernel.org>; > ath10k@lists.infradead.org; linux-wireless <linux-wireless@vger.kernel.org>; > scsi <linux-scsi@vger.kernel.org>; Linux Fbdev development list <linux- > fbdev@vger.kernel.org>; driverdevel <devel@driverdev.osuosl.org>; Linux > FS Devel <linux-fsdevel@vger.kernel.org> > Subject: Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of > repeated bytes > > Hi Alastair, > > Thanks for your patch! And thanks for your politeness :) > > On Wed, May 8, 2019 at 9:04 AM Alastair D'Silva <alastair@au1.ibm.com> > wrote: > > From: Alastair D'Silva <alastair@d-silva.org> > > > > Some buffers may only be partially filled with useful data, while the > > rest is padded (typically with 0x00 or 0xff). > > > > This patch introduces a flag to allow the supression of lines of > > repeated bytes, > > Given print_hex_dump() operates on entities of groupsize (1, 2, 4, or 8) > bytes, wouldn't it make more sense to consider repeated groups instead of > repeated bytes? Maybe, it would mean that subsequent addresses may not be a multiple of rowsize though, which is useful. > > which are replaced with '** Skipped %u bytes of value 0x%x **' > > Using a custom message instead of just "*", like "hexdump" uses, will require > preprocessing the output when recovering the original binary data by > feeding it to e.g. "xxd". > This may sound worse than it is, though, as I never got "xxd" to work without > preprocessing anyway ;-) I think showing the details of the skipped values is useful when reading the output directly. In situations where binary extracts are desired, the feature can always be disabled.
diff --git a/include/linux/printk.h b/include/linux/printk.h index d7c77ed1a4cb..938a67580d78 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -479,13 +479,18 @@ enum { DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET }; + extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii); + +#define HEXDUMP_ASCII (1 << 0) +#define HEXDUMP_SUPPRESS_REPEATED (1 << 1) + #ifdef CONFIG_PRINTK -extern void print_hex_dump(const char *level, const char *prefix_str, +extern void print_hex_dump_ext(const char *level, const char *prefix_str, int prefix_type, int rowsize, int groupsize, - const void *buf, size_t len, bool ascii); + const void *buf, size_t len, u64 flags); #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true) @@ -494,18 +499,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len); #endif /* defined(CONFIG_DYNAMIC_DEBUG) */ #else -static inline void print_hex_dump(const char *level, const char *prefix_str, +static inline void print_hex_dump_ext(const char *level, const char *prefix_str, int prefix_type, int rowsize, int groupsize, - const void *buf, size_t len, bool ascii) + const void *buf, size_t len, u64 flags) { } static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { } - #endif +static __always_inline void print_hex_dump(const char *level, + const char *prefix_str, + int prefix_type, int rowsize, + int groupsize, const void *buf, + size_t len, bool ascii) +{ + print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize, + buf, len, ascii ? HEXDUMP_ASCII : 0); +} + + #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ groupsize, buf, len, ascii) \ diff --git a/lib/hexdump.c b/lib/hexdump.c index 3943507bc0e9..d61a1e4f19fa 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, EXPORT_SYMBOL(hex_dump_to_buffer); #ifdef CONFIG_PRINTK + +/** + * Check if a buffer contains only a single byte value + * @buf: pointer to the buffer + * @len: the size of the buffer in bytes + * @val: outputs the value if if the bytes are identical + */ +static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out) +{ + size_t i; + u8 val; + + if (len <= 1) + return false; + + val = buf[0]; + + for (i = 1; i < len; i++) { + if (buf[i] != val) + return false; + } + + *val_out = val; + return true; +} + +static void announce_skipped(const char *level, const char *prefix_str, + u8 val, size_t count) +{ + if (count == 0) + return; + + printk("%s%s ** Skipped %lu bytes of value 0x%x **\n", + level, prefix_str, count, val); +} + /** - * print_hex_dump - print a text hex dump to syslog for a binary blob of data + * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal * @level: kernel log level (e.g. KERN_DEBUG) * @prefix_str: string to prefix each line with; * caller supplies trailing spaces for alignment if desired @@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * @buf: data blob to dump * @len: number of bytes in the @buf * @ascii: include ASCII after the hex output + * @flags: A bitwise OR of the following flags: + * HEXDUMP_ASCII: include ASCII after the hex output + * HEXDUMP_SUPPRESS_REPEATED: suppress repeated lines of identical + * bytes * * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump * to the kernel log at the specified kernel log level, with an optional @@ -234,22 +274,25 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * (optionally) ASCII output. * * E.g.: - * print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS, - * 16, 1, frame->data, frame->len, true); + * print_hex_dump_ext(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS, + * 16, 1, frame->data, frame->len, HEXDUMP_ASCII); * * Example output using %DUMP_PREFIX_OFFSET and 1-byte mode: * 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode: * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. */ -void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, - int rowsize, int groupsize, - const void *buf, size_t len, bool ascii) +void print_hex_dump_ext(const char *level, const char *prefix_str, + int prefix_type, int rowsize, int groupsize, + const void *buf, size_t len, u64 flags) { const u8 *ptr = buf; - int i, linelen, remaining = len; + int i, remaining = len; unsigned char *linebuf; unsigned int linebuf_len; + u8 skipped_val = 0; + size_t skipped = 0; + if (rowsize % groupsize) rowsize -= rowsize % groupsize; @@ -266,11 +309,35 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, } for (i = 0; i < len; i += rowsize) { - linelen = min(remaining, rowsize); + int linelen = min(remaining, rowsize); remaining -= rowsize; + if (flags & HEXDUMP_SUPPRESS_REPEATED) { + u8 new_val; + + if (buf_is_all(ptr + i, linelen, &new_val)) { + if (new_val == skipped_val) { + skipped += linelen; + continue; + } else { + announce_skipped(level, prefix_str, + skipped_val, skipped); + skipped_val = new_val; + skipped = linelen; + continue; + } + } + } + + if (skipped) { + announce_skipped(level, prefix_str, skipped_val, + skipped); + skipped = 0; + } + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, - linebuf, linebuf_len, ascii); + linebuf, linebuf_len, + flags & HEXDUMP_ASCII); switch (prefix_type) { case DUMP_PREFIX_ADDRESS: @@ -288,7 +355,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, kfree(linebuf); } -EXPORT_SYMBOL(print_hex_dump); +EXPORT_SYMBOL(print_hex_dump_ext); #if !defined(CONFIG_DYNAMIC_DEBUG) /** @@ -306,8 +373,8 @@ EXPORT_SYMBOL(print_hex_dump); void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { - print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1, - buf, len, true); + print_hex_dump_ext(KERN_DEBUG, prefix_str, prefix_type, 16, 1, + buf, len, HEXDUMP_ASCII); } EXPORT_SYMBOL(print_hex_dump_bytes); #endif /* !defined(CONFIG_DYNAMIC_DEBUG) */