[PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer

David Laight posted 8 patches 11 months ago
[PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer
Posted by David Laight 11 months ago
Using the same data that is passed to hex_dump_to_buffer() lets
the tests be expanded to test different input data bytes.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 lib/test_hexdump.c | 80 +++++++++-------------------------------------
 1 file changed, 15 insertions(+), 65 deletions(-)

diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 502768e56e4e..e142b11c36c6 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -3,6 +3,7 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/ctype.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -16,49 +17,6 @@ static const unsigned char data_b[] = {
 	'\x4c', '\xd1', '\x19', '\x99', '\x43', '\xb1', '\xaf', '\x0c',	/* 18 - 1f */
 };
 
-static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
-
-static const char * const test_data_1[] __initconst = {
-	"be", "32", "db", "7b", "0a", "18", "93", "b2",
-	"70", "ba", "c4", "24", "7d", "83", "34", "9b",
-	"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
-	"4c", "d1", "19", "99", "43", "b1", "af", "0c",
-};
-
-static const char * const test_data_2_le[] __initconst = {
-	"32be", "7bdb", "180a", "b293",
-	"ba70", "24c4", "837d", "9b34",
-	"9ca6", "ad31", "0f9c", "e9ac",
-	"d14c", "9919", "b143", "0caf",
-};
-
-static const char * const test_data_2_be[] __initconst = {
-	"be32", "db7b", "0a18", "93b2",
-	"70ba", "c424", "7d83", "349b",
-	"a69c", "31ad", "9c0f", "ace9",
-	"4cd1", "1999", "43b1", "af0c",
-};
-
-static const char * const test_data_4_le[] __initconst = {
-	"7bdb32be", "b293180a", "24c4ba70", "9b34837d",
-	"ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
-};
-
-static const char * const test_data_4_be[] __initconst = {
-	"be32db7b", "0a1893b2", "70bac424", "7d83349b",
-	"a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
-};
-
-static const char * const test_data_8_le[] __initconst = {
-	"b293180a7bdb32be", "9b34837d24c4ba70",
-	"e9ac0f9cad319ca6", "0cafb1439919d14c",
-};
-
-static const char * const test_data_8_be[] __initconst = {
-	"be32db7b0a1893b2", "70bac4247d83349b",
-	"a69c31ad9c0face9", "4cd1199943b1af0c",
-};
-
 #define FILL_CHAR	'#'
 
 static unsigned total_tests __initdata;
@@ -69,11 +27,9 @@ static void __init test_hexdump_prepare_test(size_t len, size_t rowsize,
 					     size_t testlen, bool ascii)
 {
 	char *p;
-	const char * const *result;
 	size_t l = len;
 	size_t gs = groupsize, rs = rowsize;
-	unsigned int i;
-	const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
+	size_t bs, i, j;
 
 	if (rs != 16 && rs != 32)
 		rs = 16;
@@ -83,26 +39,18 @@ static void __init test_hexdump_prepare_test(size_t len, size_t rowsize,
 
 	if (!is_power_of_2(gs) || gs > 8 || (len % gs != 0))
 		gs = 1;
-
-	if (gs == 8)
-		result = is_be ? test_data_8_be : test_data_8_le;
-	else if (gs == 4)
-		result = is_be ? test_data_4_be : test_data_4_le;
-	else if (gs == 2)
-		result = is_be ? test_data_2_be : test_data_2_le;
-	else
-		result = test_data_1;
+	bs = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) ? 0 : gs - 1;
 
 	/* hex dump */
 	p = test;
-	for (i = 0; i < l / gs; i++) {
-		const char *q = *result++;
-		size_t amount = strlen(q);
-
-		memcpy(p, q, amount);
-		p += amount;
-
-		*p++ = ' ';
+	for (i = 0, j = 0; i < l; i++) {
+		unsigned char b = data_b[i ^ bs];
+		*p++ = "0123456789abcdef"[b >> 4];
+		*p++ = "0123456789abcdef"[b & 15];
+		if (++j == gs) {
+			j = 0;
+			*p++ = ' ';
+		}
 	}
 	if (i)
 		p--;
@@ -113,8 +61,10 @@ static void __init test_hexdump_prepare_test(size_t len, size_t rowsize,
 			*p++ = ' ';
 		} while (p < test + rs * 2 + rs / gs + 1);
 
-		memcpy(p, data_a, l);
-		p += l;
+		for (i = 0; i < l; i++) {
+			unsigned char b = data_b[i];
+			*p++ = (isascii(b) && isprint(b)) ? b : '.';
+		}
 	}
 
 	*p = '\0';
-- 
2.39.5
Re: [PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer
Posted by Andy Shevchenko 11 months ago
On Sat, Mar 08, 2025 at 09:34:46AM +0000, David Laight wrote:
> Using the same data that is passed to hex_dump_to_buffer() lets
> the tests be expanded to test different input data bytes.

Do we really need to kill the static test cases?
Are they anyhow harmful?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer
Posted by David Laight 11 months ago
On Mon, 10 Mar 2025 10:31:10 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Mar 08, 2025 at 09:34:46AM +0000, David Laight wrote:
> > Using the same data that is passed to hex_dump_to_buffer() lets
> > the tests be expanded to test different input data bytes.  
> 
> Do we really need to kill the static test cases?
> Are they anyhow harmful?
> 

I was asked to add some extra tests for other byte values.
The static result buffers just get in the way.

They are also not necessary since the tests are comparing the output
of two (hopefully) different implementations and checking they are
the same.

	David
Re: [PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer
Posted by Andy Shevchenko 11 months ago
On Wed, Mar 12, 2025 at 07:28:11PM +0000, David Laight wrote:
> On Mon, 10 Mar 2025 10:31:10 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Mar 08, 2025 at 09:34:46AM +0000, David Laight wrote:
> > > Using the same data that is passed to hex_dump_to_buffer() lets
> > > the tests be expanded to test different input data bytes.  
> > 
> > Do we really need to kill the static test cases?
> > Are they anyhow harmful?
> 
> I was asked to add some extra tests for other byte values.

Right and thanks for doing that!

> The static result buffers just get in the way.
> 
> They are also not necessary since the tests are comparing the output
> of two (hopefully) different implementations and checking they are
> the same.

Not necessary doesn't mean harmful or working wrong. I would leave them as is
and just add a dynamic test cases on top. Static data is kinda randomised, but
at the same time it's always the same through the test runs. IIRC your dynamic
case generates the expected output and hence uses the code that also needs to
be tested strictly speaking.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH next 2/8] test_hexdump: Create test output data from the binary input data buffer
Posted by David Laight 11 months ago
On Wed, 12 Mar 2025 21:36:04 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Mar 12, 2025 at 07:28:11PM +0000, David Laight wrote:
> > On Mon, 10 Mar 2025 10:31:10 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:  
> > > On Sat, Mar 08, 2025 at 09:34:46AM +0000, David Laight wrote:  
> > > > Using the same data that is passed to hex_dump_to_buffer() lets
> > > > the tests be expanded to test different input data bytes.    
> > > 
> > > Do we really need to kill the static test cases?
> > > Are they anyhow harmful?  
> > 
> > I was asked to add some extra tests for other byte values.  
> 
> Right and thanks for doing that!
> 
> > The static result buffers just get in the way.
> > 
> > They are also not necessary since the tests are comparing the output
> > of two (hopefully) different implementations and checking they are
> > the same.  
> 
> Not necessary doesn't mean harmful or working wrong. I would leave them as is
> and just add a dynamic test cases on top. Static data is kinda randomised, but
> at the same time it's always the same through the test runs. IIRC your dynamic
> case generates the expected output and hence uses the code that also needs to
> be tested strictly speaking.
> 

The old data wasn't really randomised at all.
It tended to run a selected tests lots of times.
As an example there is no point randomly selecting between running the
'rowsize == 16' and 'rowsize == 32' tests several times when you can just
run each test once.

While that can make sense for some kinds of tests it is pointless here.
Much better to always run the same tests and to always cover the 'interesting'
cases - there are lots of very boring cases that all go through the same code
paths pretty much regardless of the implementation.
It is the corner cases that matter.

At one point I was testing all 'bufsize * len * rowsize * groupsize' tests.
Doesn't actually take long even though there are a lot of tests.
But there is just no point testing all the cases where the buffer is long
enough for the data. The boundary conditions are enough - and there are
actually under a dozen of them. The proposed tests do about 1000 just to
save working out which ones matter.

	David