From: Zhen Lei <[email protected]>
The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
the output width is fixed to 8. But we usually dump only tens or hundreds
of bytes, occasionally thousands of bytes. Therefore, the output offset
value always has a number of leading zeros, which increases the number of
bytes printed and reduces readability. Let's minimize the output width of
the offset based on the number of significant bits of its maximum value.
Before:
dump_size=36:
00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
00000020: 80 ca 2f 98
After:
dump_size=8:
0: c0 ba 89 80 00 80 ff ff
dump_size=36:
00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
20: 40 9e 29 40
dump_size=300:
000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
... ...
110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
120: 00 08 12 01 0c 40 ff ff 00 00 01 00
Signed-off-by: Zhen Lei <[email protected]>
---
lib/hexdump.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 06833d404398d74..86cb4cc3eec485a 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
const void *buf, size_t len, bool ascii)
{
const u8 *ptr = buf;
- int i, linelen, remaining = len;
+ int i, linelen, width = 0, remaining = len;
unsigned char linebuf[32 * 3 + 2 + 32 + 1];
if (rowsize != 16 && rowsize != 32)
rowsize = 16;
+ if (prefix_type == DUMP_PREFIX_OFFSET) {
+ unsigned long tmp = len - 1; /* offset start from 0, so minus 1 */
+
+ do {
+ width++;
+ tmp >>= 4;
+ } while (tmp);
+ }
+
for (i = 0; i < len; i += rowsize) {
linelen = min(remaining, rowsize);
remaining -= rowsize;
@@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
level, prefix_str, ptr + i, linebuf);
break;
case DUMP_PREFIX_OFFSET:
- printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
+ printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
break;
default:
printk("%s%s%s\n", level, prefix_str, linebuf);
--
2.34.1
Hi--
On 8/5/23 00:21, [email protected] wrote:
> From: Zhen Lei <[email protected]>
>
> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
> the output width is fixed to 8. But we usually dump only tens or hundreds
> of bytes, occasionally thousands of bytes. Therefore, the output offset
> value always has a number of leading zeros, which increases the number of
> bytes printed and reduces readability. Let's minimize the output width of
> the offset based on the number of significant bits of its maximum value.
>
> Before:
> dump_size=36:
> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
> 00000020: 80 ca 2f 98
>
> After:
> dump_size=8:
> 0: c0 ba 89 80 00 80 ff ff
>
> dump_size=36:
> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
> 20: 40 9e 29 40
>
> dump_size=300:
> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
> ... ...
> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>
> Signed-off-by: Zhen Lei <[email protected]>
> ---
> lib/hexdump.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 06833d404398d74..86cb4cc3eec485a 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
> const void *buf, size_t len, bool ascii)
> {
> const u8 *ptr = buf;
> - int i, linelen, remaining = len;
> + int i, linelen, width = 0, remaining = len;
> unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>
> if (rowsize != 16 && rowsize != 32)
> rowsize = 16;
>
> + if (prefix_type == DUMP_PREFIX_OFFSET) {
> + unsigned long tmp = len - 1; /* offset start from 0, so minus 1 */
> +
> + do {
> + width++;
> + tmp >>= 4;
> + } while (tmp);
> + }
> +
You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
Otherwise LGTM.
Reviewed-by: Randy Dunlap <[email protected]>
Thanks.
> for (i = 0; i < len; i += rowsize) {
> linelen = min(remaining, rowsize);
> remaining -= rowsize;
> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
> level, prefix_str, ptr + i, linebuf);
> break;
> case DUMP_PREFIX_OFFSET:
> - printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
> + printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
> break;
> default:
> printk("%s%s%s\n", level, prefix_str, linebuf);
--
~Randy
On 2023/8/8 6:37, Randy Dunlap wrote:
> Hi--
>
> On 8/5/23 00:21, [email protected] wrote:
>> From: Zhen Lei <[email protected]>
>>
>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>> the output width is fixed to 8. But we usually dump only tens or hundreds
>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>> value always has a number of leading zeros, which increases the number of
>> bytes printed and reduces readability. Let's minimize the output width of
>> the offset based on the number of significant bits of its maximum value.
>>
>> Before:
>> dump_size=36:
>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>> 00000020: 80 ca 2f 98
>>
>> After:
>> dump_size=8:
>> 0: c0 ba 89 80 00 80 ff ff
>>
>> dump_size=36:
>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>> 20: 40 9e 29 40
>>
>> dump_size=300:
>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>> ... ...
>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>
>> Signed-off-by: Zhen Lei <[email protected]>
>> ---
>> lib/hexdump.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>> index 06833d404398d74..86cb4cc3eec485a 100644
>> --- a/lib/hexdump.c
>> +++ b/lib/hexdump.c
>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> const void *buf, size_t len, bool ascii)
>> {
>> const u8 *ptr = buf;
>> - int i, linelen, remaining = len;
>> + int i, linelen, width = 0, remaining = len;
>> unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>
>> if (rowsize != 16 && rowsize != 32)
>> rowsize = 16;
>>
>> + if (prefix_type == DUMP_PREFIX_OFFSET) {
>> + unsigned long tmp = len - 1; /* offset start from 0, so minus 1 */
>> +
>> + do {
>> + width++;
>> + tmp >>= 4;
>> + } while (tmp);
>> + }
>> +
>
> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
for (i = 0; i < len; i += rowsize) {
"case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
following your prompt, I thought again, I can control it with the
local variable width. I will post v2 right away.
> Otherwise LGTM.
>
> Reviewed-by: Randy Dunlap <[email protected]>
> Thanks.
>
>> for (i = 0; i < len; i += rowsize) {
>> linelen = min(remaining, rowsize);
>> remaining -= rowsize;
>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>> level, prefix_str, ptr + i, linebuf);
>> break;
>> case DUMP_PREFIX_OFFSET:
>> - printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>> + printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>> break;
>> default:
>> printk("%s%s%s\n", level, prefix_str, linebuf);
>
--
Regards,
Zhen Lei
On 2023/8/8 10:42, Randy Dunlap wrote:
>
>
> On 8/7/23 18:10, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2023/8/8 6:37, Randy Dunlap wrote:
>>> Hi--
>>>
>>> On 8/5/23 00:21, [email protected] wrote:
>>>> From: Zhen Lei <[email protected]>
>>>>
>>>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>>>> the output width is fixed to 8. But we usually dump only tens or hundreds
>>>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>>>> value always has a number of leading zeros, which increases the number of
>>>> bytes printed and reduces readability. Let's minimize the output width of
>>>> the offset based on the number of significant bits of its maximum value.
>>>>
>>>> Before:
>>>> dump_size=36:
>>>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>>>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>>>> 00000020: 80 ca 2f 98
>>>>
>>>> After:
>>>> dump_size=8:
>>>> 0: c0 ba 89 80 00 80 ff ff
>>>>
>>>> dump_size=36:
>>>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>>>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>>>> 20: 40 9e 29 40
>>>>
>>>> dump_size=300:
>>>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>>>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>>>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>>>> ... ...
>>>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>>>
>>>> Signed-off-by: Zhen Lei <[email protected]>
>>>> ---
>>>> lib/hexdump.c | 13 +++++++++++--
>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>>>> index 06833d404398d74..86cb4cc3eec485a 100644
>>>> --- a/lib/hexdump.c
>>>> +++ b/lib/hexdump.c
>>>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>> const void *buf, size_t len, bool ascii)
>>>> {
>>>> const u8 *ptr = buf;
>>>> - int i, linelen, remaining = len;
>>>> + int i, linelen, width = 0, remaining = len;
>>>> unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>>>
>>>> if (rowsize != 16 && rowsize != 32)
>>>> rowsize = 16;
>>>>
>>>> + if (prefix_type == DUMP_PREFIX_OFFSET) {
>>>> + unsigned long tmp = len - 1; /* offset start from 0, so minus 1 */
>>>> +
>>>> + do {
>>>> + width++;
>>>> + tmp >>= 4;
>>>> + } while (tmp);
>>>> + }
>>>> +
>>>
>>> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
>>
>> for (i = 0; i < len; i += rowsize) {
>>
>> "case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
>> to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
>> following your prompt, I thought again, I can control it with the
>> local variable width. I will post v2 right away.
>
> Ah I see. My apologies.
It should be okay to move it into the case DUMP_PREFIX_OFFSET. The
compiler can optimize it.
>
>>> Otherwise LGTM.
>>>
>>> Reviewed-by: Randy Dunlap <[email protected]>
>>> Thanks.
>>>
>>>> for (i = 0; i < len; i += rowsize) {
>>>> linelen = min(remaining, rowsize);
>>>> remaining -= rowsize;
>>>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>>> level, prefix_str, ptr + i, linebuf);
>>>> break;
>>>> case DUMP_PREFIX_OFFSET:
>>>> - printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>>>> + printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>>>> break;
>>>> default:
>>>> printk("%s%s%s\n", level, prefix_str, linebuf);
>>>
>>
>
--
Regards,
Zhen Lei
On 8/7/23 18:10, Leizhen (ThunderTown) wrote:
>
>
> On 2023/8/8 6:37, Randy Dunlap wrote:
>> Hi--
>>
>> On 8/5/23 00:21, [email protected] wrote:
>>> From: Zhen Lei <[email protected]>
>>>
>>> The offset of case DUMP_PREFIX_OFFSET always starts from 0. Currently,
>>> the output width is fixed to 8. But we usually dump only tens or hundreds
>>> of bytes, occasionally thousands of bytes. Therefore, the output offset
>>> value always has a number of leading zeros, which increases the number of
>>> bytes printed and reduces readability. Let's minimize the output width of
>>> the offset based on the number of significant bits of its maximum value.
>>>
>>> Before:
>>> dump_size=36:
>>> 00000000: c0 ba 8c 80 00 80 ff ff 6c 93 ee 2f ee bf ff ff
>>> 00000010: 00 50 1e 98 ff 27 ff ff 01 00 00 00 00 00 00 00
>>> 00000020: 80 ca 2f 98
>>>
>>> After:
>>> dump_size=8:
>>> 0: c0 ba 89 80 00 80 ff ff
>>>
>>> dump_size=36:
>>> 00: c0 3a 91 80 00 80 ff ff 6c 93 ae 76 30 ce ff ff
>>> 10: 00 60 cd 60 7d 4e ff ff 01 00 00 00 00 00 00 00
>>> 20: 40 9e 29 40
>>>
>>> dump_size=300:
>>> 000: c0 ba 8d 80 00 80 ff ff 6c 93 ce d4 78 a7 ff ff
>>> 010: 00 00 16 18 0c 40 ff ff 01 00 00 00 00 00 00 00
>>> 020: 01 00 00 00 00 00 00 00 e8 bc 8d 80 00 80 ff ff
>>> ... ...
>>> 110: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 120: 00 08 12 01 0c 40 ff ff 00 00 01 00
>>>
>>> Signed-off-by: Zhen Lei <[email protected]>
>>> ---
>>> lib/hexdump.c | 13 +++++++++++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/hexdump.c b/lib/hexdump.c
>>> index 06833d404398d74..86cb4cc3eec485a 100644
>>> --- a/lib/hexdump.c
>>> +++ b/lib/hexdump.c
>>> @@ -263,12 +263,21 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>> const void *buf, size_t len, bool ascii)
>>> {
>>> const u8 *ptr = buf;
>>> - int i, linelen, remaining = len;
>>> + int i, linelen, width = 0, remaining = len;
>>> unsigned char linebuf[32 * 3 + 2 + 32 + 1];
>>>
>>> if (rowsize != 16 && rowsize != 32)
>>> rowsize = 16;
>>>
>>> + if (prefix_type == DUMP_PREFIX_OFFSET) {
>>> + unsigned long tmp = len - 1; /* offset start from 0, so minus 1 */
>>> +
>>> + do {
>>> + width++;
>>> + tmp >>= 4;
>>> + } while (tmp);
>>> + }
>>> +
>>
>> You could put all of that ^^^ in the case DUMP_PREFIX_OFFSET below.
>
> for (i = 0; i < len; i += rowsize) {
>
> "case DUMP_PREFIX_OFFSET" is in the loop, and moving the code above
> to the case DUMP_PREFIX_OFFSET will be calculate multiple times. But
> following your prompt, I thought again, I can control it with the
> local variable width. I will post v2 right away.
Ah I see. My apologies.
>> Otherwise LGTM.
>>
>> Reviewed-by: Randy Dunlap <[email protected]>
>> Thanks.
>>
>>> for (i = 0; i < len; i += rowsize) {
>>> linelen = min(remaining, rowsize);
>>> remaining -= rowsize;
>>> @@ -282,7 +291,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
>>> level, prefix_str, ptr + i, linebuf);
>>> break;
>>> case DUMP_PREFIX_OFFSET:
>>> - printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);
>>> + printk("%s%s%0*x: %s\n", level, prefix_str, width, i, linebuf);
>>> break;
>>> default:
>>> printk("%s%s%s\n", level, prefix_str, linebuf);
>>
>
--
~Randy