linux-kernel - Re: [PATCH v2 2/2] hexdump: Allow skipping identical lines

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4qE7tsTOvggdUET@pathway.suse.cz>
Date: Fri, 17 Jan 2025 17:27:26 +0100
From: Petr Mladek <pmladek@...e.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Jonathan Corbet <corbet@....net>,
	John Ogness <john.ogness@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] hexdump: Allow skipping identical lines

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