Re: [PATCH 2/2] hexdump: Allow skipping identical lines

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Aug 27, 2024 at 11:13:53AM +0200, Miquel Raynal wrote:
> andriy.shevchenko@linux.intel.com wrote on Mon, 26 Aug 2024 20:35:36
> +0300:
> > On Mon, Aug 26, 2024 at 06:24:16PM +0200, 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  
> > 
> > The problem here is that without offset we can't see how many lines were
> > skipped.
> 
> Yes, this is intended, I prefer to mimic userspace tools behavior.
> 
> > Two ways to solve (that come to my mind immediately, maybe more and better):
> > 1) make sure that new flag implies or expects (otherwise BUILD_BUG_ON() or so)
> >   the OFFSET to be set;
> 
> It depends what you are looking for. When I print a 2kiB page and want
> to compare the output with some other dump, I will immediately see if
> there are more or less skipped lines in the diff. When I want to just
> grab the UBI header and skip all the ff's following while asking a full
> buffer to be dumped (for kernel development reasons), the amount of
> skipped lines is not of interest to me either. Of course this is my own
> use case, but I guess there are others.
> 
> However this is true it is sometimes also useful to know where we are in
> the dump, but the hexdump helpers already include all the interesting
> bits for that through the 'prefix_type' parameter :
> 
> enum {
> 	DUMP_PREFIX_NONE,
> 	DUMP_PREFIX_ADDRESS,
> 	DUMP_PREFIX_OFFSET
> };
> 
> See https://elixir.bootlin.com/linux/v4.20.17/source/include/linux/printk.h
> 
> I anyway understand the request and will change the example with
> something more common, probably, by using one of the two other
> prefixes.
> 
> > 2) [OR] add number of lines skipped in that * line.
> 
> As mentioned above, this is not the intended output.
> 
> > Personally I prefer the 1) as I think that you tried to follow the existing
> > format of user space tools and there is a chance that there are other tools or
> > scripts that parse the dump to restore the binary contents.
> 
> Exactly. Also, just simply using the diff command over two dumps
> without being polluted by any additions on one side or the other is very
> convenient.

I got it, then provide a good examples in the cover letter / commit message,
documentation, and test cases.

After thinking more about this, if the caller asked for DUMP_PREFIX_NONE,
that's what they get if they add also SKIP flag. So, maybe here is no
problem after all :-)

-- 
With Best Regards,
Andy Shevchenko