[PATCH v7 0/3] scanf: convert self-test to KUnit

[PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

This is one of just 3 remaining "Test Module" kselftests (the others
being bitmap and printf), the rest having been converted to KUnit. In
addition to the enclosed patch, please consider this an RFC on the
removal of the "Test Module" kselftest machinery.

I tested this using:

$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v7:
- Remove redundant debug logs. (Petr Mladek)
- Drop Petr's Acked-by.
- Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
  messages. The new failure output is:

    vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
            not ok 1 " "
        # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
    vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
            not ok 2 ":"
        # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
    vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
            not ok 3 ","
        # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
    vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
            not ok 4 "-"
        # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
    vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
            not ok 5 "/"
        # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
        not ok 4 numbers_list_field_width_val_width
        # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
    vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518

- Link to v6: https://lore.kernel.org/r/20250210-scanf-kunit-convert-v6-0-4d583d07f92d@gmail.com

Changes in v6:
- s/at boot/at runtime/ for consistency with the printf series.
- Go back to kmalloc. (Geert Uytterhoeven)
- Link to v5: https://lore.kernel.org/r/20250210-scanf-kunit-convert-v5-0-8e64f3a7de99@gmail.com

Changes in v5:
- Remove extraneous trailing newlines from failure messages.
- Replace `pr_debug` with `kunit_printk`.
- Use static char arrays instead of kmalloc.
- Drop KUnit boilerplate from CONFIG_SCANF_KUNIT_TEST help text.
- Drop arch changes.
- Link to v4: https://lore.kernel.org/r/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com

Changes in v4:
- Bake `test` into various macros, greatly reducing diff noise.
- Revert control flow changes.
- Link to v3: https://lore.kernel.org/r/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com

Changes in v3:
- Reduce diff noise in lib/Makefile. (Petr Mladek)
- Split `scanf_test` into a few test cases. New output:
  : =================== scanf (10 subtests) ====================
  : [PASSED] numbers_simple
  : ====================== numbers_list  =======================
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ================== [PASSED] numbers_list ===================
  : ============ numbers_list_field_width_typemax  =============
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ======== [PASSED] numbers_list_field_width_typemax =========
  : =========== numbers_list_field_width_val_width  ============
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ======= [PASSED] numbers_list_field_width_val_width ========
  : [PASSED] numbers_slice
  : [PASSED] numbers_prefix_overflow
  : [PASSED] test_simple_strtoull
  : [PASSED] test_simple_strtoll
  : [PASSED] test_simple_strtoul
  : [PASSED] test_simple_strtol
  : ====================== [PASSED] scanf ======================
  : ============================================================
  : Testing complete. Ran 22 tests: passed: 22
  : Elapsed time: 5.517s total, 0.001s configuring, 5.440s building, 0.067s running
- Link to v2: https://lore.kernel.org/r/20250203-scanf-kunit-convert-v2-1-277a618d804e@gmail.com

Changes in v2:
- Rename lib/{test_scanf.c => scanf_kunit.c}. (Andy Shevchenko)
- Link to v1: https://lore.kernel.org/r/20250131-scanf-kunit-convert-v1-1-0976524f0eba@gmail.com

---
Tamir Duberstein (3):
      scanf: remove redundant debug logs
      scanf: convert self-test to KUnit
      scanf: break kunit into test cases

 MAINTAINERS                          |   2 +-
 lib/Kconfig.debug                    |  12 +-
 lib/Makefile                         |   2 +-
 lib/{test_scanf.c => scanf_kunit.c}  | 270 +++++++++++++++++------------------
 tools/testing/selftests/lib/Makefile |   2 +-
 tools/testing/selftests/lib/config   |   1 -
 tools/testing/selftests/lib/scanf.sh |   4 -
 7 files changed, 143 insertions(+), 150 deletions(-)
---
base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
change-id: 20250131-scanf-kunit-convert-f70dc33bb34c

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>

[PATCH v7 1/3] scanf: remove redundant debug logs

[Tamir Duberstein]

The test already prints the same information on failure; remove
redundant pr_debug() logs.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 lib/test_scanf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/test_scanf.c b/lib/test_scanf.c
index 44f8508c9d88..07444a852fd4 100644
--- a/lib/test_scanf.c
+++ b/lib/test_scanf.c
@@ -62,10 +62,8 @@ _test(check_fn fn, const void *check_data, const char *string, const char *fmt,
 
 #define _check_numbers_template(arg_fmt, expect, str, fmt, n_args, ap)		\
 do {										\
-	pr_debug("\"%s\", \"%s\" ->\n", str, fmt);				\
 	for (; n_args > 0; n_args--, expect++) {				\
 		typeof(*expect) got = *va_arg(ap, typeof(expect));		\
-		pr_debug("\t" arg_fmt "\n", got);				\
 		if (got != *expect) {						\
 			pr_warn("vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt "\n", \
 				str, fmt, *expect, got);			\
@@ -689,7 +687,6 @@ do {										\
 	total_tests++;								\
 	len = snprintf(test_buffer, BUF_SIZE, gen_fmt, expect);			\
 	got = (fn)(test_buffer, &endp, base);					\
-	pr_debug(#fn "(\"%s\", %d) -> " gen_fmt "\n", test_buffer, base, got);	\
 	if (got != (expect)) {							\
 		fail = true;							\
 		pr_warn(#fn "(\"%s\", %d): got " gen_fmt " expected " gen_fmt "\n", \

-- 
2.48.1

[PATCH v7 2/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

Convert the scanf() self-test to a KUnit test.

In the interest of keeping the patch reasonably-sized this doesn't
refactor the tests into proper parameterized tests - it's all one big
test case.

Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 MAINTAINERS                          |   2 +-
 lib/Kconfig.debug                    |  12 +-
 lib/Makefile                         |   2 +-
 lib/{test_scanf.c => scanf_kunit.c}  | 221 +++++++++++++++++------------------
 tools/testing/selftests/lib/Makefile |   2 +-
 tools/testing/selftests/lib/config   |   1 -
 tools/testing/selftests/lib/scanf.sh |   4 -
 7 files changed, 116 insertions(+), 128 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 25c86f47353d..ab1d90c66c02 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25411,8 +25411,8 @@ R:	Sergey Senozhatsky <senozhatsky@chromium.org>
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
 F:	Documentation/core-api/printk-formats.rst
+F:	lib/scanf_kunit.c
 F:	lib/test_printf.c
-F:	lib/test_scanf.c
 F:	lib/vsprintf.c
 
 VT1211 HARDWARE MONITOR DRIVER
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af972a92d06..48625139fb7f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2427,6 +2427,15 @@ config ASYNC_RAID6_TEST
 config TEST_HEXDUMP
 	tristate "Test functions located in the hexdump module at runtime"
 
+config SCANF_KUNIT_TEST
+	tristate "KUnit test scanf() family of functions at runtime" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Enable this option to test the scanf functions at runtime.
+
+	  If unsure, say N.
+
 config STRING_KUNIT_TEST
 	tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT
@@ -2443,9 +2452,6 @@ config TEST_KSTRTOX
 config TEST_PRINTF
 	tristate "Test printf() family of functions at runtime"
 
-config TEST_SCANF
-	tristate "Test scanf() family of functions at runtime"
-
 config TEST_BITMAP
 	tristate "Test bitmap_*() family of functions at runtime"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index d5cfc7afbbb8..6bdf2287878d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -85,7 +85,7 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
 obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
-obj-$(CONFIG_TEST_SCANF) += test_scanf.o
+obj-$(CONFIG_SCANF_KUNIT_TEST) += scanf_kunit.o
 
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
 ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_KASAN),yy)
diff --git a/lib/test_scanf.c b/lib/scanf_kunit.c
similarity index 82%
rename from lib/test_scanf.c
rename to lib/scanf_kunit.c
index 07444a852fd4..196a654680c4 100644
--- a/lib/test_scanf.c
+++ b/lib/scanf_kunit.c
@@ -3,10 +3,8 @@
  * Test cases for sscanf facility.
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
+#include <kunit/test.h>
 #include <linux/bitops.h>
-#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/overflow.h>
@@ -15,48 +13,34 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
-#include "../tools/testing/selftests/kselftest_module.h"
-
 #define BUF_SIZE 1024
 
-KSTM_MODULE_GLOBALS();
-static char *test_buffer __initdata;
-static char *fmt_buffer __initdata;
-static struct rnd_state rnd_state __initdata;
+static char *test_buffer;
+static char *fmt_buffer;
+static struct rnd_state rnd_state;
 
-typedef int (*check_fn)(const void *check_data, const char *string,
+typedef void (*check_fn)(struct kunit *test, const void *check_data, const char *string,
 			const char *fmt, int n_args, va_list ap);
 
-static void __scanf(4, 6) __init
-_test(check_fn fn, const void *check_data, const char *string, const char *fmt,
+static void __scanf(5, 7)
+_test(struct kunit *test, check_fn fn, const void *check_data, const char *string, const char *fmt,
 	int n_args, ...)
 {
 	va_list ap, ap_copy;
 	int ret;
 
-	total_tests++;
-
 	va_start(ap, n_args);
 	va_copy(ap_copy, ap);
 	ret = vsscanf(string, fmt, ap_copy);
 	va_end(ap_copy);
 
 	if (ret != n_args) {
-		pr_warn("vsscanf(\"%s\", \"%s\", ...) returned %d expected %d\n",
-			string, fmt, ret, n_args);
-		goto fail;
+		KUNIT_FAIL(test, "vsscanf(\"%s\", \"%s\", ...) returned %d expected %d",
+			   string, fmt, ret, n_args);
+	} else {
+		(*fn)(test, check_data, string, fmt, n_args, ap);
 	}
 
-	ret = (*fn)(check_data, string, fmt, n_args, ap);
-	if (ret)
-		goto fail;
-
-	va_end(ap);
-
-	return;
-
-fail:
-	failed_tests++;
 	va_end(ap);
 }
 
@@ -65,15 +49,14 @@ do {										\
 	for (; n_args > 0; n_args--, expect++) {				\
 		typeof(*expect) got = *va_arg(ap, typeof(expect));		\
 		if (got != *expect) {						\
-			pr_warn("vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt "\n", \
-				str, fmt, *expect, got);			\
-			return 1;						\
+			KUNIT_FAIL(test,					\
+				   "vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt, \
+				   str, fmt, *expect, got);			\
 		}								\
 	}									\
-	return 0;								\
 } while (0)
 
-static int __init check_ull(const void *check_data, const char *string,
+static void check_ull(struct kunit *test, const void *check_data, const char *string,
 			    const char *fmt, int n_args, va_list ap)
 {
 	const unsigned long long *pval = check_data;
@@ -81,7 +64,7 @@ static int __init check_ull(const void *check_data, const char *string,
 	_check_numbers_template("%llu", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_ll(const void *check_data, const char *string,
+static void check_ll(struct kunit *test, const void *check_data, const char *string,
 			   const char *fmt, int n_args, va_list ap)
 {
 	const long long *pval = check_data;
@@ -89,7 +72,7 @@ static int __init check_ll(const void *check_data, const char *string,
 	_check_numbers_template("%lld", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_ulong(const void *check_data, const char *string,
+static void check_ulong(struct kunit *test, const void *check_data, const char *string,
 			   const char *fmt, int n_args, va_list ap)
 {
 	const unsigned long *pval = check_data;
@@ -97,7 +80,7 @@ static int __init check_ulong(const void *check_data, const char *string,
 	_check_numbers_template("%lu", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_long(const void *check_data, const char *string,
+static void check_long(struct kunit *test, const void *check_data, const char *string,
 			  const char *fmt, int n_args, va_list ap)
 {
 	const long *pval = check_data;
@@ -105,7 +88,7 @@ static int __init check_long(const void *check_data, const char *string,
 	_check_numbers_template("%ld", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_uint(const void *check_data, const char *string,
+static void check_uint(struct kunit *test, const void *check_data, const char *string,
 			     const char *fmt, int n_args, va_list ap)
 {
 	const unsigned int *pval = check_data;
@@ -113,7 +96,7 @@ static int __init check_uint(const void *check_data, const char *string,
 	_check_numbers_template("%u", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_int(const void *check_data, const char *string,
+static void check_int(struct kunit *test, const void *check_data, const char *string,
 			    const char *fmt, int n_args, va_list ap)
 {
 	const int *pval = check_data;
@@ -121,7 +104,7 @@ static int __init check_int(const void *check_data, const char *string,
 	_check_numbers_template("%d", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_ushort(const void *check_data, const char *string,
+static void check_ushort(struct kunit *test, const void *check_data, const char *string,
 			       const char *fmt, int n_args, va_list ap)
 {
 	const unsigned short *pval = check_data;
@@ -129,7 +112,7 @@ static int __init check_ushort(const void *check_data, const char *string,
 	_check_numbers_template("%hu", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_short(const void *check_data, const char *string,
+static void check_short(struct kunit *test, const void *check_data, const char *string,
 			       const char *fmt, int n_args, va_list ap)
 {
 	const short *pval = check_data;
@@ -137,7 +120,7 @@ static int __init check_short(const void *check_data, const char *string,
 	_check_numbers_template("%hd", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_uchar(const void *check_data, const char *string,
+static void check_uchar(struct kunit *test, const void *check_data, const char *string,
 			       const char *fmt, int n_args, va_list ap)
 {
 	const unsigned char *pval = check_data;
@@ -145,7 +128,7 @@ static int __init check_uchar(const void *check_data, const char *string,
 	_check_numbers_template("%hhu", pval, string, fmt, n_args, ap);
 }
 
-static int __init check_char(const void *check_data, const char *string,
+static void check_char(struct kunit *test, const void *check_data, const char *string,
 			       const char *fmt, int n_args, va_list ap)
 {
 	const signed char *pval = check_data;
@@ -154,7 +137,7 @@ static int __init check_char(const void *check_data, const char *string,
 }
 
 /* Selection of interesting numbers to test, copied from test-kstrtox.c */
-static const unsigned long long numbers[] __initconst = {
+static const unsigned long long numbers[] = {
 	0x0ULL,
 	0x1ULL,
 	0x7fULL,
@@ -194,7 +177,7 @@ do {									\
 	T result = ~expect_val; /* should be overwritten */		\
 									\
 	snprintf(test_buffer, BUF_SIZE, gen_fmt, expect_val);		\
-	_test(fn, &expect_val, test_buffer, "%" scan_fmt, 1, &result);	\
+	_test(test, fn, &expect_val, test_buffer, "%" scan_fmt, 1, &result);\
 } while (0)
 
 #define simple_numbers_loop(T, gen_fmt, scan_fmt, fn)			\
@@ -212,7 +195,7 @@ do {									\
 	}								\
 } while (0)
 
-static void __init numbers_simple(void)
+static void numbers_simple(struct kunit *test)
 {
 	simple_numbers_loop(unsigned long long,	"%llu",	  "llu", check_ull);
 	simple_numbers_loop(long long,		"%lld",	  "lld", check_ll);
@@ -265,14 +248,14 @@ static void __init numbers_simple(void)
  * the raw prandom*() functions (Not mathematically rigorous!!).
  * Variabilty of length and value is more important than perfect randomness.
  */
-static u32 __init next_test_random(u32 max_bits)
+static u32 next_test_random(u32 max_bits)
 {
 	u32 n_bits = hweight32(prandom_u32_state(&rnd_state)) % (max_bits + 1);
 
 	return prandom_u32_state(&rnd_state) & GENMASK(n_bits, 0);
 }
 
-static unsigned long long __init next_test_random_ull(void)
+static unsigned long long next_test_random_ull(void)
 {
 	u32 rand1 = prandom_u32_state(&rnd_state);
 	u32 n_bits = (hweight32(rand1) * 3) % 64;
@@ -309,7 +292,7 @@ do {										\
  * updating buf_pos and returning the number of characters appended.
  * On error buf_pos is not changed and return value is 0.
  */
-static int __init __printf(4, 5)
+static int __printf(4, 5)
 append_fmt(char *buf, int *buf_pos, int buf_len, const char *val_fmt, ...)
 {
 	va_list ap;
@@ -331,7 +314,7 @@ append_fmt(char *buf, int *buf_pos, int buf_len, const char *val_fmt, ...)
  * Convenience function to append the field delimiter string
  * to both the value string and format string buffers.
  */
-static void __init append_delim(char *str_buf, int *str_buf_pos, int str_buf_len,
+static void append_delim(char *str_buf, int *str_buf_pos, int str_buf_len,
 				char *fmt_buf, int *fmt_buf_pos, int fmt_buf_len,
 				const char *delim_str)
 {
@@ -342,7 +325,7 @@ static void __init append_delim(char *str_buf, int *str_buf_pos, int str_buf_len
 #define test_array_8(fn, check_data, string, fmt, arr)				\
 do {										\
 	BUILD_BUG_ON(ARRAY_SIZE(arr) != 8);					\
-	_test(fn, check_data, string, fmt, 8,					\
+	_test(test, fn, check_data, string, fmt, 8,				\
 		&(arr)[0], &(arr)[1], &(arr)[2], &(arr)[3],			\
 		&(arr)[4], &(arr)[5], &(arr)[6], &(arr)[7]);			\
 } while (0)
@@ -396,7 +379,7 @@ do {										\
 	test_array_8(fn, expect, test_buffer, fmt_buffer, result);		\
 } while (0)
 
-static void __init numbers_list_ll(const char *delim)
+static void numbers_list_ll(struct kunit *test, const char *delim)
 {
 	numbers_list_8(unsigned long long, "%llu",   delim, "llu", check_ull);
 	numbers_list_8(long long,	   "%lld",   delim, "lld", check_ll);
@@ -406,7 +389,7 @@ static void __init numbers_list_ll(const char *delim)
 	numbers_list_8(long long,	   "0x%llx", delim, "lli", check_ll);
 }
 
-static void __init numbers_list_l(const char *delim)
+static void numbers_list_l(struct kunit *test, const char *delim)
 {
 	numbers_list_8(unsigned long,	   "%lu",    delim, "lu", check_ulong);
 	numbers_list_8(long,		   "%ld",    delim, "ld", check_long);
@@ -416,7 +399,7 @@ static void __init numbers_list_l(const char *delim)
 	numbers_list_8(long,		   "0x%lx",  delim, "li", check_long);
 }
 
-static void __init numbers_list_d(const char *delim)
+static void numbers_list_d(struct kunit *test, const char *delim)
 {
 	numbers_list_8(unsigned int,	   "%u",     delim, "u", check_uint);
 	numbers_list_8(int,		   "%d",     delim, "d", check_int);
@@ -426,7 +409,7 @@ static void __init numbers_list_d(const char *delim)
 	numbers_list_8(int,		   "0x%x",   delim, "i", check_int);
 }
 
-static void __init numbers_list_h(const char *delim)
+static void numbers_list_h(struct kunit *test, const char *delim)
 {
 	numbers_list_8(unsigned short,	   "%hu",    delim, "hu", check_ushort);
 	numbers_list_8(short,		   "%hd",    delim, "hd", check_short);
@@ -436,7 +419,7 @@ static void __init numbers_list_h(const char *delim)
 	numbers_list_8(short,		   "0x%hx",  delim, "hi", check_short);
 }
 
-static void __init numbers_list_hh(const char *delim)
+static void numbers_list_hh(struct kunit *test, const char *delim)
 {
 	numbers_list_8(unsigned char,	   "%hhu",   delim, "hhu", check_uchar);
 	numbers_list_8(signed char,	   "%hhd",   delim, "hhd", check_char);
@@ -446,16 +429,16 @@ static void __init numbers_list_hh(const char *delim)
 	numbers_list_8(signed char,	   "0x%hhx", delim, "hhi", check_char);
 }
 
-static void __init numbers_list(const char *delim)
+static void numbers_list(struct kunit *test, const char *delim)
 {
-	numbers_list_ll(delim);
-	numbers_list_l(delim);
-	numbers_list_d(delim);
-	numbers_list_h(delim);
-	numbers_list_hh(delim);
+	numbers_list_ll(test, delim);
+	numbers_list_l(test, delim);
+	numbers_list_d(test, delim);
+	numbers_list_h(test, delim);
+	numbers_list_hh(test, delim);
 }
 
-static void __init numbers_list_field_width_ll(const char *delim)
+static void numbers_list_field_width_ll(struct kunit *test, const char *delim)
 {
 	numbers_list_fix_width(unsigned long long, "%llu",   delim, 20, "llu", check_ull);
 	numbers_list_fix_width(long long,	   "%lld",   delim, 20, "lld", check_ll);
@@ -465,7 +448,7 @@ static void __init numbers_list_field_width_ll(const char *delim)
 	numbers_list_fix_width(long long,	   "0x%llx", delim, 18, "lli", check_ll);
 }
 
-static void __init numbers_list_field_width_l(const char *delim)
+static void numbers_list_field_width_l(struct kunit *test, const char *delim)
 {
 #if BITS_PER_LONG == 64
 	numbers_list_fix_width(unsigned long,	"%lu",	     delim, 20, "lu", check_ulong);
@@ -484,7 +467,7 @@ static void __init numbers_list_field_width_l(const char *delim)
 #endif
 }
 
-static void __init numbers_list_field_width_d(const char *delim)
+static void numbers_list_field_width_d(struct kunit *test, const char *delim)
 {
 	numbers_list_fix_width(unsigned int,	"%u",	     delim, 10, "u", check_uint);
 	numbers_list_fix_width(int,		"%d",	     delim, 11, "d", check_int);
@@ -494,7 +477,7 @@ static void __init numbers_list_field_width_d(const char *delim)
 	numbers_list_fix_width(int,		"0x%x",	     delim, 10, "i", check_int);
 }
 
-static void __init numbers_list_field_width_h(const char *delim)
+static void numbers_list_field_width_h(struct kunit *test, const char *delim)
 {
 	numbers_list_fix_width(unsigned short,	"%hu",	     delim, 5, "hu", check_ushort);
 	numbers_list_fix_width(short,		"%hd",	     delim, 6, "hd", check_short);
@@ -504,7 +487,7 @@ static void __init numbers_list_field_width_h(const char *delim)
 	numbers_list_fix_width(short,		"0x%hx",     delim, 6, "hi", check_short);
 }
 
-static void __init numbers_list_field_width_hh(const char *delim)
+static void numbers_list_field_width_hh(struct kunit *test, const char *delim)
 {
 	numbers_list_fix_width(unsigned char,	"%hhu",	     delim, 3, "hhu", check_uchar);
 	numbers_list_fix_width(signed char,	"%hhd",	     delim, 4, "hhd", check_char);
@@ -518,16 +501,16 @@ static void __init numbers_list_field_width_hh(const char *delim)
  * List of numbers separated by delim. Each field width specifier is the
  * maximum possible digits for the given type and base.
  */
-static void __init numbers_list_field_width_typemax(const char *delim)
+static void numbers_list_field_width_typemax(struct kunit *test, const char *delim)
 {
-	numbers_list_field_width_ll(delim);
-	numbers_list_field_width_l(delim);
-	numbers_list_field_width_d(delim);
-	numbers_list_field_width_h(delim);
-	numbers_list_field_width_hh(delim);
+	numbers_list_field_width_ll(test, delim);
+	numbers_list_field_width_l(test, delim);
+	numbers_list_field_width_d(test, delim);
+	numbers_list_field_width_h(test, delim);
+	numbers_list_field_width_hh(test, delim);
 }
 
-static void __init numbers_list_field_width_val_ll(const char *delim)
+static void numbers_list_field_width_val_ll(struct kunit *test, const char *delim)
 {
 	numbers_list_val_width(unsigned long long, "%llu",   delim, "llu", check_ull);
 	numbers_list_val_width(long long,	   "%lld",   delim, "lld", check_ll);
@@ -537,7 +520,7 @@ static void __init numbers_list_field_width_val_ll(const char *delim)
 	numbers_list_val_width(long long,	   "0x%llx", delim, "lli", check_ll);
 }
 
-static void __init numbers_list_field_width_val_l(const char *delim)
+static void numbers_list_field_width_val_l(struct kunit *test, const char *delim)
 {
 	numbers_list_val_width(unsigned long,	"%lu",	     delim, "lu", check_ulong);
 	numbers_list_val_width(long,		"%ld",	     delim, "ld", check_long);
@@ -547,7 +530,7 @@ static void __init numbers_list_field_width_val_l(const char *delim)
 	numbers_list_val_width(long,		"0x%lx",     delim, "li", check_long);
 }
 
-static void __init numbers_list_field_width_val_d(const char *delim)
+static void numbers_list_field_width_val_d(struct kunit *test, const char *delim)
 {
 	numbers_list_val_width(unsigned int,	"%u",	     delim, "u", check_uint);
 	numbers_list_val_width(int,		"%d",	     delim, "d", check_int);
@@ -557,7 +540,7 @@ static void __init numbers_list_field_width_val_d(const char *delim)
 	numbers_list_val_width(int,		"0x%x",	     delim, "i", check_int);
 }
 
-static void __init numbers_list_field_width_val_h(const char *delim)
+static void numbers_list_field_width_val_h(struct kunit *test, const char *delim)
 {
 	numbers_list_val_width(unsigned short,	"%hu",	     delim, "hu", check_ushort);
 	numbers_list_val_width(short,		"%hd",	     delim, "hd", check_short);
@@ -567,7 +550,7 @@ static void __init numbers_list_field_width_val_h(const char *delim)
 	numbers_list_val_width(short,		"0x%hx",     delim, "hi", check_short);
 }
 
-static void __init numbers_list_field_width_val_hh(const char *delim)
+static void numbers_list_field_width_val_hh(struct kunit *test, const char *delim)
 {
 	numbers_list_val_width(unsigned char,	"%hhu",	     delim, "hhu", check_uchar);
 	numbers_list_val_width(signed char,	"%hhd",	     delim, "hhd", check_char);
@@ -581,13 +564,13 @@ static void __init numbers_list_field_width_val_hh(const char *delim)
  * List of numbers separated by delim. Each field width specifier is the
  * exact length of the corresponding value digits in the string being scanned.
  */
-static void __init numbers_list_field_width_val_width(const char *delim)
+static void numbers_list_field_width_val_width(struct kunit *test, const char *delim)
 {
-	numbers_list_field_width_val_ll(delim);
-	numbers_list_field_width_val_l(delim);
-	numbers_list_field_width_val_d(delim);
-	numbers_list_field_width_val_h(delim);
-	numbers_list_field_width_val_hh(delim);
+	numbers_list_field_width_val_ll(test, delim);
+	numbers_list_field_width_val_l(test, delim);
+	numbers_list_field_width_val_d(test, delim);
+	numbers_list_field_width_val_h(test, delim);
+	numbers_list_field_width_val_hh(test, delim);
 }
 
 /*
@@ -596,9 +579,9 @@ static void __init numbers_list_field_width_val_width(const char *delim)
  * of digits. For example the hex values c0,3,bf01,303 would have a
  * string representation of "c03bf01303" and extracted with "%2x%1x%4x%3x".
  */
-static void __init numbers_slice(void)
+static void numbers_slice(struct kunit *test)
 {
-	numbers_list_field_width_val_width("");
+	numbers_list_field_width_val_width(test, "");
 }
 
 #define test_number_prefix(T, str, scan_fmt, expect0, expect1, n_args, fn)	\
@@ -606,14 +589,14 @@ do {										\
 	const T expect[2] = { expect0, expect1 };				\
 	T result[2] = { (T)~expect[0], (T)~expect[1] };				\
 										\
-	_test(fn, &expect, str, scan_fmt, n_args, &result[0], &result[1]);	\
+	_test(test, fn, &expect, str, scan_fmt, n_args, &result[0], &result[1]);\
 } while (0)
 
 /*
  * Number prefix is >= field width.
  * Expected behaviour is derived from testing userland sscanf.
  */
-static void __init numbers_prefix_overflow(void)
+static void numbers_prefix_overflow(struct kunit *test)
 {
 	/*
 	 * Negative decimal with a field of width 1, should quit scanning
@@ -682,24 +665,17 @@ do {										\
 	T got;									\
 	char *endp;								\
 	int len;								\
-	bool fail = false;							\
 										\
-	total_tests++;								\
 	len = snprintf(test_buffer, BUF_SIZE, gen_fmt, expect);			\
 	got = (fn)(test_buffer, &endp, base);					\
 	if (got != (expect)) {							\
-		fail = true;							\
-		pr_warn(#fn "(\"%s\", %d): got " gen_fmt " expected " gen_fmt "\n", \
-			test_buffer, base, got, expect);			\
+		KUNIT_FAIL(test, #fn "(\"%s\", %d): got " gen_fmt " expected " gen_fmt, \
+			   test_buffer, base, got, expect);			\
 	} else if (endp != test_buffer + len) {					\
-		fail = true;							\
-		pr_warn(#fn "(\"%s\", %d) startp=0x%px got endp=0x%px expected 0x%px\n", \
-			test_buffer, base, test_buffer,				\
-			test_buffer + len, endp);				\
+		KUNIT_FAIL(test, #fn "(\"%s\", %d) startp=0x%px got endp=0x%px expected 0x%px", \
+			   test_buffer, base, test_buffer,			\
+			   test_buffer + len, endp);				\
 	}									\
-										\
-	if (fail)								\
-		failed_tests++;							\
 } while (0)
 
 #define test_simple_strtoxx(T, fn, gen_fmt, base)				\
@@ -715,7 +691,7 @@ do {										\
 	}									\
 } while (0)
 
-static void __init test_simple_strtoull(void)
+static void test_simple_strtoull(struct kunit *test)
 {
 	test_simple_strtoxx(unsigned long long, simple_strtoull, "%llu",   10);
 	test_simple_strtoxx(unsigned long long, simple_strtoull, "%llu",   0);
@@ -724,7 +700,7 @@ static void __init test_simple_strtoull(void)
 	test_simple_strtoxx(unsigned long long, simple_strtoull, "0x%llx", 0);
 }
 
-static void __init test_simple_strtoll(void)
+static void test_simple_strtoll(struct kunit *test)
 {
 	test_simple_strtoxx(long long, simple_strtoll, "%lld",	 10);
 	test_simple_strtoxx(long long, simple_strtoll, "%lld",	 0);
@@ -733,7 +709,7 @@ static void __init test_simple_strtoll(void)
 	test_simple_strtoxx(long long, simple_strtoll, "0x%llx", 0);
 }
 
-static void __init test_simple_strtoul(void)
+static void test_simple_strtoul(struct kunit *test)
 {
 	test_simple_strtoxx(unsigned long, simple_strtoul, "%lu",   10);
 	test_simple_strtoxx(unsigned long, simple_strtoul, "%lu",   0);
@@ -742,7 +718,7 @@ static void __init test_simple_strtoul(void)
 	test_simple_strtoxx(unsigned long, simple_strtoul, "0x%lx", 0);
 }
 
-static void __init test_simple_strtol(void)
+static void test_simple_strtol(struct kunit *test)
 {
 	test_simple_strtoxx(long, simple_strtol, "%ld",   10);
 	test_simple_strtoxx(long, simple_strtol, "%ld",   0);
@@ -752,35 +728,35 @@ static void __init test_simple_strtol(void)
 }
 
 /* Selection of common delimiters/separators between numbers in a string. */
-static const char * const number_delimiters[] __initconst = {
+static const char * const number_delimiters[] = {
 	" ", ":", ",", "-", "/",
 };
 
-static void __init test_numbers(void)
+static void test_numbers(struct kunit *test)
 {
 	int i;
 
 	/* String containing only one number. */
-	numbers_simple();
+	numbers_simple(test);
 
 	/* String with multiple numbers separated by delimiter. */
 	for (i = 0; i < ARRAY_SIZE(number_delimiters); i++) {
-		numbers_list(number_delimiters[i]);
+		numbers_list(test, number_delimiters[i]);
 
 		/* Field width may be longer than actual field digits. */
-		numbers_list_field_width_typemax(number_delimiters[i]);
+		numbers_list_field_width_typemax(test, number_delimiters[i]);
 
 		/* Each field width exactly length of actual field digits. */
-		numbers_list_field_width_val_width(number_delimiters[i]);
+		numbers_list_field_width_val_width(test, number_delimiters[i]);
 	}
 
 	/* Slice continuous sequence of digits using field widths. */
-	numbers_slice();
+	numbers_slice(test);
 
-	numbers_prefix_overflow();
+	numbers_prefix_overflow(test);
 }
 
-static void __init selftest(void)
+static void scanf_test(struct kunit *test)
 {
 	test_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
 	if (!test_buffer)
@@ -794,18 +770,29 @@ static void __init selftest(void)
 
 	prandom_seed_state(&rnd_state, 3141592653589793238ULL);
 
-	test_numbers();
+	test_numbers(test);
 
-	test_simple_strtoull();
-	test_simple_strtoll();
-	test_simple_strtoul();
-	test_simple_strtol();
+	test_simple_strtoull(test);
+	test_simple_strtoll(test);
+	test_simple_strtoul(test);
+	test_simple_strtol(test);
 
 	kfree(fmt_buffer);
 	kfree(test_buffer);
 }
 
-KSTM_MODULE_LOADERS(test_scanf);
+static struct kunit_case scanf_test_cases[] = {
+	KUNIT_CASE(scanf_test),
+	{}
+};
+
+static struct kunit_suite scanf_test_suite = {
+	.name = "scanf",
+	.test_cases = scanf_test_cases,
+};
+
+kunit_test_suite(scanf_test_suite);
+
 MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.cirrus.com>");
 MODULE_DESCRIPTION("Test cases for sscanf facility");
 MODULE_LICENSE("GPL v2");
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
index c52fe3ad8e98..4afda556151f 100644
--- a/tools/testing/selftests/lib/Makefile
+++ b/tools/testing/selftests/lib/Makefile
@@ -4,5 +4,5 @@
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
 all:
 
-TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh scanf.sh
+TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh
 include ../lib.mk
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index dc15aba8d0a3..80b55a504868 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -1,5 +1,4 @@
 CONFIG_TEST_PRINTF=m
-CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_PRIME_NUMBERS=m
 CONFIG_TEST_BITOPS=m
diff --git a/tools/testing/selftests/lib/scanf.sh b/tools/testing/selftests/lib/scanf.sh
deleted file mode 100755
index b59b8ba561c3..000000000000
--- a/tools/testing/selftests/lib/scanf.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-# Tests the scanf infrastructure using test_scanf kernel module.
-$(dirname $0)/../kselftest/module.sh "scanf" test_scanf

-- 
2.48.1

[PATCH v7 3/3] scanf: break kunit into test cases

[Tamir Duberstein]

Use `suite_init` and move some tests into `scanf_test_cases`. This
gives us nicer output in the event of a failure.

Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 lib/scanf_kunit.c | 94 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/lib/scanf_kunit.c b/lib/scanf_kunit.c
index 196a654680c4..e7b40c8eb53e 100644
--- a/lib/scanf_kunit.c
+++ b/lib/scanf_kunit.c
@@ -4,14 +4,10 @@
  */
 
 #include <kunit/test.h>
-#include <linux/bitops.h>
-#include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/overflow.h>
-#include <linux/printk.h>
 #include <linux/prandom.h>
 #include <linux/slab.h>
-#include <linux/string.h>
+#include <linux/sprintf.h>
 
 #define BUF_SIZE 1024
 
@@ -49,9 +45,9 @@ do {										\
 	for (; n_args > 0; n_args--, expect++) {				\
 		typeof(*expect) got = *va_arg(ap, typeof(expect));		\
 		if (got != *expect) {						\
-			KUNIT_FAIL(test,					\
-				   "vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt, \
-				   str, fmt, *expect, got);			\
+			KUNIT_FAIL_AND_ABORT(test,				\
+					     "vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt, \
+					     str, fmt, *expect, got);		\
 		}								\
 	}									\
 } while (0)
@@ -429,8 +425,11 @@ static void numbers_list_hh(struct kunit *test, const char *delim)
 	numbers_list_8(signed char,	   "0x%hhx", delim, "hhi", check_char);
 }
 
-static void numbers_list(struct kunit *test, const char *delim)
+static void numbers_list(struct kunit *test)
 {
+	const char * const *param = test->param_value;
+	const char *delim = *param;
+
 	numbers_list_ll(test, delim);
 	numbers_list_l(test, delim);
 	numbers_list_d(test, delim);
@@ -501,8 +500,11 @@ static void numbers_list_field_width_hh(struct kunit *test, const char *delim)
  * List of numbers separated by delim. Each field width specifier is the
  * maximum possible digits for the given type and base.
  */
-static void numbers_list_field_width_typemax(struct kunit *test, const char *delim)
+static void numbers_list_field_width_typemax(struct kunit *test)
 {
+	const char * const *param = test->param_value;
+	const char *delim = *param;
+
 	numbers_list_field_width_ll(test, delim);
 	numbers_list_field_width_l(test, delim);
 	numbers_list_field_width_d(test, delim);
@@ -564,8 +566,11 @@ static void numbers_list_field_width_val_hh(struct kunit *test, const char *deli
  * List of numbers separated by delim. Each field width specifier is the
  * exact length of the corresponding value digits in the string being scanned.
  */
-static void numbers_list_field_width_val_width(struct kunit *test, const char *delim)
+static void numbers_list_field_width_val_width(struct kunit *test)
 {
+	const char * const *param = test->param_value;
+	const char *delim = *param;
+
 	numbers_list_field_width_val_ll(test, delim);
 	numbers_list_field_width_val_l(test, delim);
 	numbers_list_field_width_val_d(test, delim);
@@ -581,7 +586,12 @@ static void numbers_list_field_width_val_width(struct kunit *test, const char *d
  */
 static void numbers_slice(struct kunit *test)
 {
-	numbers_list_field_width_val_width(test, "");
+	const char *delim = "";
+
+	KUNIT_ASSERT_PTR_EQ(test, test->param_value, NULL);
+	test->param_value = &delim;
+
+	numbers_list_field_width_val_width(test);
 }
 
 #define test_number_prefix(T, str, scan_fmt, expect0, expect1, n_args, fn)	\
@@ -732,62 +742,60 @@ static const char * const number_delimiters[] = {
 	" ", ":", ",", "-", "/",
 };
 
-static void test_numbers(struct kunit *test)
+static void number_delimiter_param_desc(const char * const *param,
+					   char *desc)
 {
-	int i;
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "\"%s\"", *param);
+}
 
-	/* String containing only one number. */
-	numbers_simple(test);
+KUNIT_ARRAY_PARAM(number_delimiters, number_delimiters, number_delimiter_param_desc);
 
+static struct kunit_case scanf_test_cases[] = {
+	KUNIT_CASE(numbers_simple),
 	/* String with multiple numbers separated by delimiter. */
-	for (i = 0; i < ARRAY_SIZE(number_delimiters); i++) {
-		numbers_list(test, number_delimiters[i]);
-
-		/* Field width may be longer than actual field digits. */
-		numbers_list_field_width_typemax(test, number_delimiters[i]);
-
-		/* Each field width exactly length of actual field digits. */
-		numbers_list_field_width_val_width(test, number_delimiters[i]);
-	}
-
+	KUNIT_CASE_PARAM(numbers_list, number_delimiters_gen_params),
+	/* Field width may be longer than actual field digits. */
+	KUNIT_CASE_PARAM(numbers_list_field_width_typemax, number_delimiters_gen_params),
+	/* Each field width exactly length of actual field digits. */
+	KUNIT_CASE_PARAM(numbers_list_field_width_val_width, number_delimiters_gen_params),
 	/* Slice continuous sequence of digits using field widths. */
-	numbers_slice(test);
+	KUNIT_CASE(numbers_slice),
+	KUNIT_CASE(numbers_prefix_overflow),
 
-	numbers_prefix_overflow(test);
-}
+	KUNIT_CASE(test_simple_strtoull),
+	KUNIT_CASE(test_simple_strtoll),
+	KUNIT_CASE(test_simple_strtoul),
+	KUNIT_CASE(test_simple_strtol),
+	{}
+};
 
-static void scanf_test(struct kunit *test)
+static int scanf_suite_init(struct kunit_suite *suite)
 {
 	test_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
 	if (!test_buffer)
-		return;
+		return -ENOMEM;
 
 	fmt_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
 	if (!fmt_buffer) {
 		kfree(test_buffer);
-		return;
+		return -ENOMEM;
 	}
 
 	prandom_seed_state(&rnd_state, 3141592653589793238ULL);
 
-	test_numbers(test);
-
-	test_simple_strtoull(test);
-	test_simple_strtoll(test);
-	test_simple_strtoul(test);
-	test_simple_strtol(test);
+	return 0;
+}
 
+static void scanf_suite_exit(struct kunit_suite *suite)
+{
 	kfree(fmt_buffer);
 	kfree(test_buffer);
 }
 
-static struct kunit_case scanf_test_cases[] = {
-	KUNIT_CASE(scanf_test),
-	{}
-};
-
 static struct kunit_suite scanf_test_suite = {
 	.name = "scanf",
+	.suite_init = scanf_suite_init,
+	.suite_exit = scanf_suite_exit,
 	.test_cases = scanf_test_cases,
 };
 

-- 
2.48.1

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:
> This is one of just 3 remaining "Test Module" kselftests (the others
> being bitmap and printf), the rest having been converted to KUnit. In
> addition to the enclosed patch, please consider this an RFC on the
> removal of the "Test Module" kselftest machinery.
> 
> I tested this using:
> 
> $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Changes in v7:
> - Remove redundant debug logs. (Petr Mladek)
> - Drop Petr's Acked-by.
> - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
>   messages. The new failure output is:

It would be good if you put into cover letter, or even in the respectful patch
the example of the error report for the old code and new code that it will be
clear how it changes.

>     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
>             not ok 1 " "
>         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
>     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
>             not ok 2 ":"
>         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
>     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
>             not ok 3 ","
>         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
>     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
>             not ok 4 "-"
>         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
>     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
>             not ok 5 "/"
>         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
>         not ok 4 numbers_list_field_width_val_width
>         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
>     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> The test already prints the same information on failure; remove
> redundant pr_debug() logs.

...

>  #define _check_numbers_template(arg_fmt, expect, str, fmt, n_args, ap)		\
>  do {										\
> -	pr_debug("\"%s\", \"%s\" ->\n", str, fmt);				\

What *if* the n_args == 0 here?

>  	for (; n_args > 0; n_args--, expect++) {				\
>  		typeof(*expect) got = *va_arg(ap, typeof(expect));		\
> -		pr_debug("\t" arg_fmt "\n", got);				\
>  		if (got != *expect) {						\
>  			pr_warn("vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt "\n", \
>  				str, fmt, *expect, got);			\
> @@ -689,7 +687,6 @@ do {										\
>  	total_tests++;								\
>  	len = snprintf(test_buffer, BUF_SIZE, gen_fmt, expect);			\
>  	got = (fn)(test_buffer, &endp, base);					\
> -	pr_debug(#fn "(\"%s\", %d) -> " gen_fmt "\n", test_buffer, base, got);	\
>  	if (got != (expect)) {							\
>  		fail = true;							\
>  		pr_warn(#fn "(\"%s\", %d): got " gen_fmt " expected " gen_fmt "\n", \

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:
> > This is one of just 3 remaining "Test Module" kselftests (the others
> > being bitmap and printf), the rest having been converted to KUnit. In
> > addition to the enclosed patch, please consider this an RFC on the
> > removal of the "Test Module" kselftest machinery.
> >
> > I tested this using:
> >
> > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > Changes in v7:
> > - Remove redundant debug logs. (Petr Mladek)
> > - Drop Petr's Acked-by.
> > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
> >   messages. The new failure output is:
>
> It would be good if you put into cover letter, or even in the respectful patch
> the example of the error report for the old code and new code that it will be
> clear how it changes.
>
> >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> >             not ok 1 " "
> >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> >     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> >             not ok 2 ":"
> >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> >     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> >             not ok 3 ","
> >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> >     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> >             not ok 4 "-"
> >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> >     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> >             not ok 5 "/"
> >         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
> >         not ok 4 numbers_list_field_width_val_width
> >         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
> >     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518

Makes sense. As you can see the error report for the new code is
included here. I'll add the old code's error report if I have to
respin v8.

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 10:42 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> > The test already prints the same information on failure; remove
> > redundant pr_debug() logs.
>
> ...
>
> >  #define _check_numbers_template(arg_fmt, expect, str, fmt, n_args, ap)               \
> >  do {                                                                         \
> > -     pr_debug("\"%s\", \"%s\" ->\n", str, fmt);                              \
>
> What *if* the n_args == 0 here?

Then there's no assertion in this block, so the test cannot possibly fail here.

> >       for (; n_args > 0; n_args--, expect++) {                                \
> >               typeof(*expect) got = *va_arg(ap, typeof(expect));              \
> > -             pr_debug("\t" arg_fmt "\n", got);                               \
> >               if (got != *expect) {                                           \
> >                       pr_warn("vsscanf(\"%s\", \"%s\", ...) expected " arg_fmt " got " arg_fmt "\n", \
> >                               str, fmt, *expect, got);                        \
> > @@ -689,7 +687,6 @@ do {                                                                              \
> >       total_tests++;                                                          \
> >       len = snprintf(test_buffer, BUF_SIZE, gen_fmt, expect);                 \
> >       got = (fn)(test_buffer, &endp, base);                                   \
> > -     pr_debug(#fn "(\"%s\", %d) -> " gen_fmt "\n", test_buffer, base, got);  \
> >       if (got != (expect)) {                                                  \
> >               fail = true;                                                    \
> >               pr_warn(#fn "(\"%s\", %d): got " gen_fmt " expected " gen_fmt "\n", \
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote:
> On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:

...

> > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
> > >   messages. The new failure output is:
> >
> > It would be good if you put into cover letter, or even in the respectful patch
> > the example of the error report for the old code and new code that it will be
> > clear how it changes.
> >
> > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > >             not ok 1 " "
> > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> > >             not ok 2 ":"
> > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> > >             not ok 3 ","
> > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> > >             not ok 4 "-"
> > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> > >             not ok 5 "/"
> > >         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
> > >         not ok 4 numbers_list_field_width_val_width
> > >         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518
> 
> Makes sense. As you can see the error report for the new code is
> included here. I'll add the old code's error report if I have to
> respin v8.

At a bare minimum. can you add in the reply to this email?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote:
> > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:
>
> ...
>
> > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
> > > >   messages. The new failure output is:
> > >
> > > It would be good if you put into cover letter, or even in the respectful patch
> > > the example of the error report for the old code and new code that it will be
> > > clear how it changes.
> > >
> > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > >             not ok 1 " "
> > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > >     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> > > >             not ok 2 ":"
> > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > >     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> > > >             not ok 3 ","
> > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > >     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> > > >             not ok 4 "-"
> > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > >     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> > > >             not ok 5 "/"
> > > >         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
> > > >         not ok 4 numbers_list_field_width_val_width
> > > >         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > >     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518
> >
> > Makes sense. As you can see the error report for the new code is
> > included here. I'll add the old code's error report if I have to
> > respin v8.
>
> At a bare minimum. can you add in the reply to this email?

Oh, sure:

On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote:
>
> [...]
>
> [  383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881
> [  383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970
> [  383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430
> [  383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073
> [  383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741
> [  383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936
> [  383.106398] test_scanf: failed 6 out of 2545 tests

This is from https://lore.kernel.org/all/Z6s6WsIw3VCGys6K@pathway.suse.cz/
(doesn't load for me, seems lore is having problems).

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 10:50:33AM -0500, Tamir Duberstein wrote:
> On Tue, Feb 11, 2025 at 10:42 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> > > The test already prints the same information on failure; remove
> > > redundant pr_debug() logs.

...

> > > -     pr_debug("\"%s\", \"%s\" ->\n", str, fmt);                              \
> >
> > What *if* the n_args == 0 here?
> 
> Then there's no assertion in this block, so the test cannot possibly fail here.

Correct, but I'm talking about this in a scope of the removed debug print.
I.o.w. how would we even know that this was the case?

(I'm not objecting removal, what I want from you is to have a descriptive and
 explanatory commit message that's answers to "why is this needed?" and "why is
 it safe to do?")

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 10:58 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 10:50:33AM -0500, Tamir Duberstein wrote:
> > On Tue, Feb 11, 2025 at 10:42 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> > > > The test already prints the same information on failure; remove
> > > > redundant pr_debug() logs.
>
> ...
>
> > > > -     pr_debug("\"%s\", \"%s\" ->\n", str, fmt);                              \
> > >
> > > What *if* the n_args == 0 here?
> >
> > Then there's no assertion in this block, so the test cannot possibly fail here.
>
> Correct, but I'm talking about this in a scope of the removed debug print.
> I.o.w. how would we even know that this was the case?
>
> (I'm not objecting removal, what I want from you is to have a descriptive and
>  explanatory commit message that's answers to "why is this needed?" and "why is
>  it safe to do?")

The true answer to "why is this needed" is Petr requested it in
https://lore.kernel.org/all/Z6s2eqh0jkYHntUL@pathway.suse.cz/ (again,
lore is having issues):

On Tue, Feb 11, 2025 at 6:37 AM Petr Mladek <pmladek@suse.com> wrote:
>
> [...]
>
> But when thinking more about it. I think that even pr_debug() is not
> the right solution.
>
> IMHO, we really want to print these details only when the test fails.
>
> Best Regards,
> Petr

The commit message already answers "why is it safe to do":

> The test already prints the same information on failure; remove
> redundant pr_debug() logs.

Perhaps what you're asking for is an assertion to be added if n_args
== 0? I think that would make sense. Does it belong in this series?

Tamir

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 11:02:59AM -0500, Tamir Duberstein wrote:
> On Tue, Feb 11, 2025 at 10:58 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 11, 2025 at 10:50:33AM -0500, Tamir Duberstein wrote:
> > > On Tue, Feb 11, 2025 at 10:42 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> > > > > The test already prints the same information on failure; remove
> > > > > redundant pr_debug() logs.

...

> > > > > -     pr_debug("\"%s\", \"%s\" ->\n", str, fmt);                              \
> > > >
> > > > What *if* the n_args == 0 here?
> > >
> > > Then there's no assertion in this block, so the test cannot possibly fail here.
> >
> > Correct, but I'm talking about this in a scope of the removed debug print.
> > I.o.w. how would we even know that this was the case?
> >
> > (I'm not objecting removal, what I want from you is to have a descriptive and
> >  explanatory commit message that's answers to "why is this needed?" and "why is
> >  it safe to do?")
> 
> The true answer to "why is this needed" is Petr requested it in
> https://lore.kernel.org/all/Z6s2eqh0jkYHntUL@pathway.suse.cz/ (again,
> lore is having issues):
> 
> On Tue, Feb 11, 2025 at 6:37 AM Petr Mladek <pmladek@suse.com> wrote:

[...]

> > But when thinking more about it. I think that even pr_debug() is not
> > the right solution.
> >
> > IMHO, we really want to print these details only when the test fails.
> >
> > Best Regards,
> > Petr
> 
> The commit message already answers "why is it safe to do":

Not really. It answers that "why is it safe to do when test case fails?".

> > The test already prints the same information on failure; remove
> > redundant pr_debug() logs.
> 
> Perhaps what you're asking for is an assertion to be added if n_args
> == 0? I think that would make sense. Does it belong in this series?

I don't know if it's possible case. I don't know if we need an assertion.
Please, research.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Andy Shevchenko]

On Tue, Feb 11, 2025 at 10:57:11AM -0500, Tamir Duberstein wrote:
> On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote:
> > > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:

...

> > > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
> > > > >   messages. The new failure output is:
> > > >
> > > > It would be good if you put into cover letter, or even in the respectful patch
> > > > the example of the error report for the old code and new code that it will be
> > > > clear how it changes.
> > > >
> > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > >             not ok 1 " "
> > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > >     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> > > > >             not ok 2 ":"
> > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > >     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> > > > >             not ok 3 ","
> > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > >     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> > > > >             not ok 4 "-"
> > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > >     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> > > > >             not ok 5 "/"
> > > > >         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
> > > > >         not ok 4 numbers_list_field_width_val_width
> > > > >         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > >     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518
> > >
> > > Makes sense. As you can see the error report for the new code is
> > > included here. I'll add the old code's error report if I have to
> > > respin v8.
> >
> > At a bare minimum. can you add in the reply to this email?
> 
> Oh, sure:
> 
> On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > [...]
> >
> > [  383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881
> > [  383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970
> > [  383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430
> > [  383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073
> > [  383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741
> > [  383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936

> > [  383.106398] test_scanf: failed 6 out of 2545 tests

Is it me who cut something or the above missing this information (total tests)?
If the latter, how are we supposed to answer to the question if the failed test
is from new bunch of cases I hypothetically added or regression of the existing
ones? Without this it seems like I need to go through all failures. OTOH it may
be needed anyway as failing test case needs an investigation.

> This is from https://lore.kernel.org/all/Z6s6WsIw3VCGys6K@pathway.suse.cz/
> (doesn't load for me, seems lore is having problems).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 12:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 10:57:11AM -0500, Tamir Duberstein wrote:
> > On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote:
> > > > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote:
>
> ...
>
> > > > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse
> > > > > >   messages. The new failure output is:
> > > > >
> > > > > It would be good if you put into cover letter, or even in the respectful patch
> > > > > the example of the error report for the old code and new code that it will be
> > > > > clear how it changes.
> > > > >
> > > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > > >             not ok 1 " "
> > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > > >     vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> > > > > >             not ok 2 ":"
> > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > > >     vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> > > > > >             not ok 3 ","
> > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > > >     vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> > > > > >             not ok 4 "-"
> > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > > >     vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> > > > > >             not ok 5 "/"
> > > > > >         # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5
> > > > > >         not ok 4 numbers_list_field_width_val_width
> > > > > >         # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92
> > > > > >     vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518
> > > >
> > > > Makes sense. As you can see the error report for the new code is
> > > > included here. I'll add the old code's error report if I have to
> > > > respin v8.
> > >
> > > At a bare minimum. can you add in the reply to this email?
> >
> > Oh, sure:
> >
> > On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote:
> > >
> > > [...]
> > >
> > > [  383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881
> > > [  383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970
> > > [  383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430
> > > [  383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073
> > > [  383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741
> > > [  383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936
>
> > > [  383.106398] test_scanf: failed 6 out of 2545 tests
>
> Is it me who cut something or the above missing this information (total tests)?
> If the latter, how are we supposed to answer to the question if the failed test
> is from new bunch of cases I hypothetically added or regression of the existing
> ones? Without this it seems like I need to go through all failures. OTOH it may
> be needed anyway as failing test case needs an investigation.

I assume you mean missing from the new output. Yeah, KUnit doesn't do
this counting. Instead you get the test name in the failure message:

> > > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > > >             not ok 1 " "
> > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92

I think maybe you're saying: what if I add a new assertion (rather
than a new test case), and I start getting failure reports - how do I
know if the reporter is running old or new test code?

In an ideal world the message above would give you all the information
you need by including the line number from the test. This doesn't
quite work out in this case because of the various test helper
functions; you end up with a line number in the test helper rather
than in the test itself. We could fix that by passing around __FILE__
and __LINE__ (probably by wrapping the test helpers in a macro). What
do you think?

Re: [PATCH v7 1/3] scanf: remove redundant debug logs

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 12:16 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 11, 2025 at 11:02:59AM -0500, Tamir Duberstein wrote:
> > On Tue, Feb 11, 2025 at 10:58 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Feb 11, 2025 at 10:50:33AM -0500, Tamir Duberstein wrote:
> > > > On Tue, Feb 11, 2025 at 10:42 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Tue, Feb 11, 2025 at 10:13:37AM -0500, Tamir Duberstein wrote:
> > > > > > The test already prints the same information on failure; remove
> > > > > > redundant pr_debug() logs.
>
> ...
>
> > > > > > -     pr_debug("\"%s\", \"%s\" ->\n", str, fmt);                              \
> > > > >
> > > > > What *if* the n_args == 0 here?
> > > >
> > > > Then there's no assertion in this block, so the test cannot possibly fail here.
> > >
> > > Correct, but I'm talking about this in a scope of the removed debug print.
> > > I.o.w. how would we even know that this was the case?
> > >
> > > (I'm not objecting removal, what I want from you is to have a descriptive and
> > >  explanatory commit message that's answers to "why is this needed?" and "why is
> > >  it safe to do?")
> >
> > The true answer to "why is this needed" is Petr requested it in
> > https://lore.kernel.org/all/Z6s2eqh0jkYHntUL@pathway.suse.cz/ (again,
> > lore is having issues):
> >
> > On Tue, Feb 11, 2025 at 6:37 AM Petr Mladek <pmladek@suse.com> wrote:
>
> [...]
>
> > > But when thinking more about it. I think that even pr_debug() is not
> > > the right solution.
> > >
> > > IMHO, we really want to print these details only when the test fails.
> > >
> > > Best Regards,
> > > Petr
> >
> > The commit message already answers "why is it safe to do":
>
> Not really. It answers that "why is it safe to do when test case fails?".
>
> > > The test already prints the same information on failure; remove
> > > redundant pr_debug() logs.
> >
> > Perhaps what you're asking for is an assertion to be added if n_args
> > == 0? I think that would make sense. Does it belong in this series?
>
> I don't know if it's possible case. I don't know if we need an assertion.
> Please, research.

Such an assertion is not necessary. `_check_numbers_template` is
called from `check_{ull,ll,ulong,long,uint,int,ushort,short,uchar,char}`
which are in turn called from `_test`:

> if (ret != n_args) {
>   KUNIT_FAIL(test, "vsscanf(\"%s\", \"%s\", ...) returned %d expected %d", string, fmt, ret, n_args);
> } else {
>   (*fn)(test, check_data, string, fmt, n_args, ap); // <-- `fn` is `check_*`.
> }

So `n_args` comes from the test expectation, and it's already checked
against the return value of `vsscanf`.

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> > Is it me who cut something or the above missing this information (total tests)?
> > If the latter, how are we supposed to answer to the question if the failed test
> > is from new bunch of cases I hypothetically added or regression of the existing
> > ones? Without this it seems like I need to go through all failures. OTOH it may
> > be needed anyway as failing test case needs an investigation.
>
> I assume you mean missing from the new output. Yeah, KUnit doesn't do
> this counting. Instead you get the test name in the failure message:
>
> > > > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > > > >             not ok 1 " "
> > > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
>
> I think maybe you're saying: what if I add a new assertion (rather
> than a new test case), and I start getting failure reports - how do I
> know if the reporter is running old or new test code?
>
> In an ideal world the message above would give you all the information
> you need by including the line number from the test. This doesn't
> quite work out in this case because of the various test helper
> functions; you end up with a line number in the test helper rather
> than in the test itself. We could fix that by passing around __FILE__
> and __LINE__ (probably by wrapping the test helpers in a macro). What
> do you think?

I gave this a try locally, and it produced this output:

>     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
>         not ok 1 " "
>     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
>         not ok 2 ":"
>     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
>         not ok 3 ","
>     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
>         not ok 4 "-"
>     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
>         not ok 5 "/"

Andy, Petr: what do you think? I've added this (and the original
output, as you requested) to the cover letter for when I reroll v8
(not before next week).

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Petr Mladek]

On Wed 2025-02-12 11:54:52, Tamir Duberstein wrote:
> On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > > Is it me who cut something or the above missing this information (total tests)?
> > > If the latter, how are we supposed to answer to the question if the failed test
> > > is from new bunch of cases I hypothetically added or regression of the existing
> > > ones? Without this it seems like I need to go through all failures. OTOH it may
> > > be needed anyway as failing test case needs an investigation.
> >
> > I assume you mean missing from the new output. Yeah, KUnit doesn't do
> > this counting. Instead you get the test name in the failure message:
> >
> > > > > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > > > > >             not ok 1 " "
> > > > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> >
> > I think maybe you're saying: what if I add a new assertion (rather
> > than a new test case), and I start getting failure reports - how do I
> > know if the reporter is running old or new test code?
> >
> > In an ideal world the message above would give you all the information
> > you need by including the line number from the test. This doesn't
> > quite work out in this case because of the various test helper
> > functions; you end up with a line number in the test helper rather
> > than in the test itself. We could fix that by passing around __FILE__
> > and __LINE__ (probably by wrapping the test helpers in a macro). What
> > do you think?

I am not sure how many changes are needed to wrap the test helpers in
a macro.

> I gave this a try locally, and it produced this output:
> 
> >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> >         not ok 1 " "
> >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> >         not ok 2 ":"
> >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> >         not ok 3 ","
> >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> >         not ok 4 "-"
> >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> >         not ok 5 "/"

But I really like that the error message shows the exact line of the
caller. IMHO, it is very helpful in this module. I like it.

IMHO, it also justifies removing the pr_debug() messages (currently 1st patch).

> Andy, Petr: what do you think? I've added this (and the original
> output, as you requested) to the cover letter for when I reroll v8
> (not before next week).

I suggest, to do the switch into macros in the 1st patch.
Remove the obsolete pr_debug() lines in 2nd patch.
Plus two more patches switching the module to kunit test.

I am personally fine with this change.

Best Regards,
Petr

Re: [PATCH v7 0/3] scanf: convert self-test to KUnit

[Tamir Duberstein]

On Fri, Feb 14, 2025 at 8:33 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Wed 2025-02-12 11:54:52, Tamir Duberstein wrote:
> > On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > >
> > > > Is it me who cut something or the above missing this information (total tests)?
> > > > If the latter, how are we supposed to answer to the question if the failed test
> > > > is from new bunch of cases I hypothetically added or regression of the existing
> > > > ones? Without this it seems like I need to go through all failures. OTOH it may
> > > > be needed anyway as failing test case needs an investigation.
> > >
> > > I assume you mean missing from the new output. Yeah, KUnit doesn't do
> > > this counting. Instead you get the test name in the failure message:
> > >
> > > > > > > > >     vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > > > > > > > >             not ok 1 " "
> > > > > > > > >         # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92
> > >
> > > I think maybe you're saying: what if I add a new assertion (rather
> > > than a new test case), and I start getting failure reports - how do I
> > > know if the reporter is running old or new test code?
> > >
> > > In an ideal world the message above would give you all the information
> > > you need by including the line number from the test. This doesn't
> > > quite work out in this case because of the various test helper
> > > functions; you end up with a line number in the test helper rather
> > > than in the test itself. We could fix that by passing around __FILE__
> > > and __LINE__ (probably by wrapping the test helpers in a macro). What
> > > do you think?
>
> I am not sure how many changes are needed to wrap the test helpers in
> a macro.
>
> > I gave this a try locally, and it produced this output:
> >
> > >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > > lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334
> > >         not ok 1 " "
> > >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > > lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28
> > >         not ok 2 ":"
> > >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > > lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614
> > >         not ok 3 ","
> > >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > > lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381
> > >         not ok 4 "-"
> > >     # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94
> > > lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328
> > >         not ok 5 "/"
>
> But I really like that the error message shows the exact line of the
> caller. IMHO, it is very helpful in this module. I like it.
>
> IMHO, it also justifies removing the pr_debug() messages (currently 1st patch).
>
> > Andy, Petr: what do you think? I've added this (and the original
> > output, as you requested) to the cover letter for when I reroll v8
> > (not before next week).
>
> I suggest, to do the switch into macros in the 1st patch.
> Remove the obsolete pr_debug() lines in 2nd patch.
> Plus two more patches switching the module to kunit test.
>
> I am personally fine with this change.
>
> Best Regards,
> Petr

Thanks Petr. I'll send v8 now, then.