[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z2rYBc7QTTvviT3s@smile.fi.intel.com>
Date: Tue, 24 Dec 2024 17:49:25 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Petr Mladek <pmladek@...e.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 0/2] hexdump: Allow skipping identical lines
On Tue, Dec 24, 2024 at 12:56:26PM +0100, Miquel Raynal wrote:
> On 27/08/2024 at 16:29:18 +03, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> > On Tue, Aug 27, 2024 at 11:01:47AM +0200, Miquel Raynal wrote:
> >> andriy.shevchenko@...ux.intel.com wrote on Mon, 26 Aug 2024 20:32:20
> >> +0300:
> >> > On Mon, Aug 26, 2024 at 06:24:14PM +0200, Miquel Raynal wrote:
...
> >> > Also here is the formal NAK till the series gains the test cases.
> >>
> >> What test cases are you talking about?
> >
> > Anything meaningful you come up with to show that the printed data is
> > what it's expected. The module has a complimentary test case,
> > lib/test_hexdump.c. Without changes in that file, there is no go
> > to what ever golden ideas you have.
>
> I had a look. The tests never test the content of the kernel buffer,
> while this is the only part that my changes have an impact on.
So, it means that after your change there will be a deviation between the core
function that dumps into a buffer and one that prints message into the kernel
buffer. Moreover it lefts seq_hex_dump() out of the picture.
I think you need to start from modifying hex_dump_to_buffer() to have a
functionality you want (note, there are cases in the kernel that use
hex_dump_to_buffer() for formatting messages in the kernel buffer and they
might want to have the same functionality to be available.
> These tests verify the hex_dump_to_buffer() logic, but never how it is used
> through the print_hex_dump_*() helpers.
I haven't checked and don't remember for sure, but KUnit rings a bell that it
might be possible to test the actual kernel output. (However, after the above
modifications been made it won't be needed anymore as test_hexdump.c will be
extended to support new feature.)
> In this series I am just enabling a new way to print the content of the
> buffer, like for instance enabling a prefix, which is not directly
> related to the core implementation of hexdump.
>
> Hence, I am sorry, but I will disregard this request unless someone
> comes up with a working idea which is worth the effort, considering the
> minimum impact of this change and the fact that it is mostly (if not
> only) for debugging purposes and will most likely never reach users.
I'm sorry, but my NAK still stands. No tests — no go.
And it does not matter if it's only for debugging or for ABI, we require test
cases for the lib/ changes. We don't know and don't care much about how these
new features will be utilized (the requirement here is to have a user for it,
so you might need to consider to convert one of the existing user to use a new
feature, besides the [updated] test cases).
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists