Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions

From: Andy Shevchenko
Date: Wed Dec 09 2015 - 18:09:53 EST

Next message: Paul E. McKenney: "[PATCH v2 tip/core/rcu 09/14] rcu: Make cpu_needs_another_gp() be bool"
Previous message: Paul E. McKenney: "[PATCH v2 tip/core/rcu 0/14] Miscellaneous fixes for 4.5"
In reply to: Joe Perches: "Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions"
Next in thread: Joe Perches: "Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Dec 10, 2015 at 12:11 AM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@xxxxxxxxxxx> wrote:
>> There are use cases for dumping buffers with specific endian types
>> for 2, 4, and 8 byte variables in arrays.
>>
>> Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such.
>>
>> Rename groupsize to groupflags in the functions and add documentation
>> to the kernel-doc to describe the use of the DUMP_TYPE_ enum.
>
> One comment below and my tag
> Acked-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

Couple of additional notes. (Tag still in power)

1. There is similar to print_hex_dump() function in seq_file. Separate patch?
2. What to do with ASCII part? Shall we print it as byte stream?

And one more style comment below.

>
>>
>> Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>
>> ---
>> include/linux/printk.h | 23 +++++++++++++++--------
>> lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
>> 2 files changed, 53 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/linux/printk.h b/include/linux/printk.h
>
>> index 9729565..aac2943 100644
>> --- a/include/linux/printk.h
>> +++ b/include/linux/printk.h
>> @@ -424,12 +424,19 @@ enum {
>> DUMP_PREFIX_ADDRESS,
>> DUMP_PREFIX_OFFSET
>> };
>> +
>> +enum {
>> + DUMP_TYPE_CPU = 0,
>> + DUMP_TYPE_LE = BIT(30),
>> + DUMP_TYPE_BE = BIT(31)
>> +};
>> +
>> extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
>> - int groupsize, char *linebuf, size_t linebuflen,
>> + int groupflags, char *linebuf, size_t linebuflen,
>> bool ascii);
>> #ifdef CONFIG_PRINTK
>> extern void print_hex_dump(const char *level, const char *prefix_str,
>> - int prefix_type, int rowsize, int groupsize,
>> + int prefix_type, int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii);
>> #if defined(CONFIG_DYNAMIC_DEBUG)
>> #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
>> @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
>> #endif /* defined(CONFIG_DYNAMIC_DEBUG) */
>> #else
>> static inline void print_hex_dump(const char *level, const char *prefix_str,
>> - int prefix_type, int rowsize, int groupsize,
>> + int prefix_type, int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> }
>> @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
>>
>> #if defined(CONFIG_DYNAMIC_DEBUG)
>> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii) \
>> + groupflags, buf, len, ascii) \
>> dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii)
>> + groupflags, buf, len, ascii)
>> #elif defined(DEBUG)
>> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii) \
>> + groupflags, buf, len, ascii) \
>> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \
>> - groupsize, buf, len, ascii)
>> + groupflags, buf, len, ascii)
>> #else
>> static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
>> - int rowsize, int groupsize,
>> + int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> }
>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>> index 992457b..f44608e 100644
>> --- a/lib/hexdump.c
>> +++ b/lib/hexdump.c
>> @@ -80,7 +80,9 @@ EXPORT_SYMBOL(bin2hex);
>> * @buf: data blob to dump
>> * @len: number of bytes in the @buf
>> * @rowsize: number of bytes to print per line; must be 16 or 32
>> - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
>> + * of @buf, otherwise %DUMP_TYPE_CPU is used
>> * @linebuf: where to put the converted data
>> * @linebuflen: total size of @linebuf, including space for terminating NUL
>> * @ascii: include ASCII after the hex output
>> @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex);
>> * (excluding the terminating NUL) which would have been written to the final
>> * string if enough space had been available.
>> */
>> -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags,
>> char *linebuf, size_t linebuflen, bool ascii)
>> {
>> const u8 *ptr = buf;
>> @@ -114,6 +116,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> int j, lx = 0;
>> int ascii_column;
>> int ret;
>> + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE);

Can you move this upper to first or second position in the definition block?

>>
>> if (rowsize != 16 && rowsize != 32)
>> rowsize = 16;
>> @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u64 *ptr8 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u64 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le64(ptr8 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be64(ptr8 + j);
>> + else
>> + val = get_unaligned(ptr8 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%16.16llx", j ? " " : "",
>> - get_unaligned(ptr8 + j));
>> + "%s%16.16llx", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u32 *ptr4 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u32 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le32(ptr4 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be32(ptr4 + j);
>> + else
>> + val = get_unaligned(ptr4 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%8.8x", j ? " " : "",
>> - get_unaligned(ptr4 + j));
>> + "%s%8.8x", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
>> const u16 *ptr2 = buf;
>>
>> for (j = 0; j < ngroups; j++) {
>> + u16 val;
>> +
>> + if (groupflags & DUMP_TYPE_LE)
>> + val = get_unaligned_le16(ptr2 + j);
>> + else if (groupflags & DUMP_TYPE_BE)
>> + val = get_unaligned_be16(ptr2 + j);
>> + else
>> + val = get_unaligned(ptr2 + j);
>> ret = snprintf(linebuf + lx, linebuflen - lx,
>> - "%s%4.4x", j ? " " : "",
>> - get_unaligned(ptr2 + j));
>> + "%s%4.4x", j ? " " : "", val);
>> if (ret >= linebuflen - lx)
>> goto overflow1;
>> lx += ret;
>> @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
>> * @prefix_type: controls whether prefix of an offset, address, or none
>> * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
>> * @rowsize: number of bytes to print per line; must be 16 or 32
>> - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>
>> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1)
>
> Duplicate.
>
>> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions
>> + * of @buf, otherwise %DUMP_TYPE_CPU is used
>> * @buf: data blob to dump
>> * @len: number of bytes in the @buf
>> * @ascii: include ASCII after the hex output
>> @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
>> * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
>> */
>> void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> - int rowsize, int groupsize,
>> + int rowsize, int groupflags,
>> const void *buf, size_t len, bool ascii)
>> {
>> const u8 *ptr = buf;
>> @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> linelen = min(remaining, rowsize);
>> remaining -= rowsize;
>>
>> - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
>> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags,
>> linebuf, sizeof(linebuf), ascii);
>>
>> switch (prefix_type) {
>> --
>> 2.6.3.368.gf34be46
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

--
With Best Regards,
Andy Shevchenko
--
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/

Next message: Paul E. McKenney: "[PATCH v2 tip/core/rcu 09/14] rcu: Make cpu_needs_another_gp() be bool"
Previous message: Paul E. McKenney: "[PATCH v2 tip/core/rcu 0/14] Miscellaneous fixes for 4.5"
In reply to: Joe Perches: "Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions"
Next in thread: Joe Perches: "Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]