On Mon 2025-01-13 14:35:41, Andy Shevchenko wrote:
> On Fri, Jan 10, 2025 at 07:42:05PM +0100, Miquel Raynal wrote:
> > When dumping long buffers (especially for debug purposes) it may be very
> > convenient to sometimes avoid spitting all the lines of the buffer if
> > the lines are identical. Typically on embedded devices, the console
> > would be wired to a UART running at 115200 bauds, which makes the dumps
> > very (very) slow. In this case, having a flag to avoid printing
> > duplicated lines is handy.
> >
> > Example of a made up repetitive output:
> > 0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb
> >
> > Same but with the flag enabled:
> > 0f 53 63 47 56 55 78 7a aa b7 8c ff ff ff ff ff
> > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > *
> > ff ff ff ff ff ff ff ff ff ff ff ff 01 2a 39 eb
>
> Still thinking that it's not okay to leave the cases where hex_dump_to_buffer()
> is being used for the similar. I would expect that to be modified as well.
> As told in v1 thread this can be achieved using a context data, instead of
> providing zillion fields, one of which may be a kind of CRC32 checksum that
> makes this work without any additional allocation.
>
> But I won't prevent you to go with this if you get a blessing from other
> PRINTK/PRINTF maintainers/reviewers.
Honestly, I never felt as a maintainer of the hexdump API.
I reviewed patches when time permitted but the changes always went
in by Andrew ;-)
Also I do not know the history of the two APIs. It seems that
hex_dump_to_buffer() is capable of writing more lines but
it seems to be primary used to fill one line.
This might explain why it does not handle the prefix...
=> hex_dump_to_buffer() is not much useful for dumping more
lines because they would be hard to analyze without the prefix,
...
=> print_hex_dump() is the API for dumping more lines
IMHO, it is perfectly fine to add support for skipping identical lines
only to print_hex_dump(). And I would go even further and replace
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)
with
void print_hex_dump(const char *level, const char *prefix_str,
enum hex_dump_type,
int rowsize, int groupsize,
const void *buf, size_t len)
and combine all the flags into the one enum:
enum hex_dump_type {
DUMP_HEX_ONLY = 0,
DUMP_HEX_AND_ASCII = BIT(1),
DUMP_PREFIX_ADDRESS = BIT(2),
DUMP_PREFIX_OFFSET = BIT(3),
DUMP_SKIP_IDENTICAL_LINES = BIT(4),
};
How does that sound, please?
Best Regards,
Petr
