This is one of just 3 remaining "Test Module" kselftests (the others being bitmap and scanf), the rest having been converted to KUnit.
I tested this using:
$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf
I have also sent out a series converting scanf[0].
Link: https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@g... [0]
Signed-off-by: Tamir Duberstein tamird@gmail.com --- Changes in v2: - Incorporate code review from prior work[0] by Arpitha Raghunandan. - Link to v1: https://lore.kernel.org/r/20250204-printf-kunit-convert-v1-0-ecf1b846a4de@gm...
Link: https://lore.kernel.org/lkml/20200817043028.76502-1-98.arpi@gmail.com/t/#u [0]
--- Tamir Duberstein (2): printf: convert self-test to KUnit printf: break kunit into test cases
Documentation/core-api/printk-formats.rst | 2 +- MAINTAINERS | 2 +- arch/m68k/configs/amiga_defconfig | 1 - arch/m68k/configs/apollo_defconfig | 1 - arch/m68k/configs/atari_defconfig | 1 - arch/m68k/configs/bvme6000_defconfig | 1 - arch/m68k/configs/hp300_defconfig | 1 - arch/m68k/configs/mac_defconfig | 1 - arch/m68k/configs/multi_defconfig | 1 - arch/m68k/configs/mvme147_defconfig | 1 - arch/m68k/configs/mvme16x_defconfig | 1 - arch/m68k/configs/q40_defconfig | 1 - arch/m68k/configs/sun3_defconfig | 1 - arch/m68k/configs/sun3x_defconfig | 1 - arch/powerpc/configs/ppc64_defconfig | 1 - lib/Kconfig.debug | 20 +- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 400 ++++++++++++------------------ tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 20 files changed, 182 insertions(+), 262 deletions(-) --- base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20250131-printf-kunit-convert-fd4012aa2ec6
Best regards,
Convert the printf() 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.
Acked-by: Geert Uytterhoeven geert@linux-m68k.org # m68k Signed-off-by: Tamir Duberstein tamird@gmail.com --- Documentation/core-api/printk-formats.rst | 2 +- MAINTAINERS | 2 +- arch/m68k/configs/amiga_defconfig | 1 - arch/m68k/configs/apollo_defconfig | 1 - arch/m68k/configs/atari_defconfig | 1 - arch/m68k/configs/bvme6000_defconfig | 1 - arch/m68k/configs/hp300_defconfig | 1 - arch/m68k/configs/mac_defconfig | 1 - arch/m68k/configs/multi_defconfig | 1 - arch/m68k/configs/mvme147_defconfig | 1 - arch/m68k/configs/mvme16x_defconfig | 1 - arch/m68k/configs/q40_defconfig | 1 - arch/m68k/configs/sun3_defconfig | 1 - arch/m68k/configs/sun3x_defconfig | 1 - arch/powerpc/configs/ppc64_defconfig | 1 - lib/Kconfig.debug | 20 +++- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 164 +++++++++++++++--------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 20 files changed, 104 insertions(+), 104 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index ecccc0473da9..0d9461bd6964 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -661,7 +661,7 @@ Do *not* use it from C. Thanks ======
-If you add other %p extensions, please extend <lib/test_printf.c> with +If you add other %p extensions, please extend <lib/printf_kunit.c> with one or more test cases, if at all feasible.
Thank you for your cooperation and attention. diff --git a/MAINTAINERS b/MAINTAINERS index 896a307fa065..57e366d356bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25368,7 +25368,7 @@ 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/test_printf.c +F: lib/printf_kunit.c F: lib/test_scanf.c F: lib/vsprintf.c
diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig index dbf2ea561c85..e8c5e08fb6ec 100644 --- a/arch/m68k/configs/amiga_defconfig +++ b/arch/m68k/configs/amiga_defconfig @@ -622,7 +622,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/apollo_defconfig b/arch/m68k/configs/apollo_defconfig index b0fd199cc0a4..dcaddfc675de 100644 --- a/arch/m68k/configs/apollo_defconfig +++ b/arch/m68k/configs/apollo_defconfig @@ -579,7 +579,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/atari_defconfig b/arch/m68k/configs/atari_defconfig index bb5b2d3b6c10..53a91c62d415 100644 --- a/arch/m68k/configs/atari_defconfig +++ b/arch/m68k/configs/atari_defconfig @@ -599,7 +599,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/bvme6000_defconfig b/arch/m68k/configs/bvme6000_defconfig index 8315a13bab73..ea2f84907c4e 100644 --- a/arch/m68k/configs/bvme6000_defconfig +++ b/arch/m68k/configs/bvme6000_defconfig @@ -571,7 +571,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/hp300_defconfig b/arch/m68k/configs/hp300_defconfig index 350370657e5f..7b1cc9f597bf 100644 --- a/arch/m68k/configs/hp300_defconfig +++ b/arch/m68k/configs/hp300_defconfig @@ -581,7 +581,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig index f942b4755702..831695e766a8 100644 --- a/arch/m68k/configs/mac_defconfig +++ b/arch/m68k/configs/mac_defconfig @@ -598,7 +598,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig index b1eaad02efab..901d8d357218 100644 --- a/arch/m68k/configs/multi_defconfig +++ b/arch/m68k/configs/multi_defconfig @@ -685,7 +685,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/mvme147_defconfig b/arch/m68k/configs/mvme147_defconfig index 6309a4442bb3..2212bb7877e2 100644 --- a/arch/m68k/configs/mvme147_defconfig +++ b/arch/m68k/configs/mvme147_defconfig @@ -571,7 +571,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/mvme16x_defconfig b/arch/m68k/configs/mvme16x_defconfig index 3feb0731f814..a87b455049e8 100644 --- a/arch/m68k/configs/mvme16x_defconfig +++ b/arch/m68k/configs/mvme16x_defconfig @@ -572,7 +572,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/q40_defconfig b/arch/m68k/configs/q40_defconfig index ea04b1b0da7d..411e555c7e07 100644 --- a/arch/m68k/configs/q40_defconfig +++ b/arch/m68k/configs/q40_defconfig @@ -588,7 +588,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/sun3_defconfig b/arch/m68k/configs/sun3_defconfig index f52d9af92153..e095fa159d05 100644 --- a/arch/m68k/configs/sun3_defconfig +++ b/arch/m68k/configs/sun3_defconfig @@ -568,7 +568,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/m68k/configs/sun3x_defconfig b/arch/m68k/configs/sun3x_defconfig index f348447824da..11eb37864ac6 100644 --- a/arch/m68k/configs/sun3x_defconfig +++ b/arch/m68k/configs/sun3x_defconfig @@ -569,7 +569,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig index 465eb96c755e..05018898d282 100644 --- a/arch/powerpc/configs/ppc64_defconfig +++ b/arch/powerpc/configs/ppc64_defconfig @@ -444,7 +444,6 @@ CONFIG_TEST_HEXDUMP=m CONFIG_STRING_SELFTEST=m CONFIG_TEST_STRING_HELPERS=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1af972a92d06..4834cac1eb8f 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST config TEST_HEXDUMP tristate "Test functions located in the hexdump module at runtime"
+config PRINTF_KUNIT_TEST + tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable this option to test the printf functions at boot. + + KUnit tests run during boot and output the results to the debug log + in TAP format (http://testanything.org/). Only useful for kernel devs + running the KUnit test harness, and not intended for inclusion into a + production build. + + For more information on KUnit and unit tests in general please refer + to the KUnit documentation in Documentation/dev-tools/kunit/. + + If unsure, say N. + config STRING_KUNIT_TEST tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS depends on KUNIT @@ -2440,9 +2457,6 @@ config STRING_HELPERS_KUNIT_TEST config TEST_KSTRTOX tristate "Test kstrto*() family of functions at runtime"
-config TEST_PRINTF - tristate "Test printf() family of functions at runtime" - config TEST_SCANF tristate "Test scanf() family of functions at runtime"
diff --git a/lib/Makefile b/lib/Makefile index d5cfc7afbbb8..844665b1f0e7 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -84,7 +84,7 @@ obj-$(CONFIG_TEST_SORT) += test_sort.o 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_PRINTF_KUNIT_TEST) += printf_kunit.o obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/test_printf.c b/lib/printf_kunit.c similarity index 89% rename from lib/test_printf.c rename to lib/printf_kunit.c index 59dbe4f9a4cb..fe6f4df92dd5 100644 --- a/lib/test_printf.c +++ b/lib/printf_kunit.c @@ -5,7 +5,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/init.h> +#include <kunit/test.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/printk.h> @@ -25,8 +25,6 @@
#include <linux/property.h>
-#include "../tools/testing/selftests/kselftest_module.h" - #define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,72 +35,71 @@ block \ __diag_pop();
-KSTM_MODULE_GLOBALS(); +static char *test_buffer; +static char *alloced_buffer; + +static struct kunit *kunittest;
-static char *test_buffer __initdata; -static char *alloced_buffer __initdata; +#define tc_fail(fmt, ...) \ + KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
-static int __printf(4, 0) __init +static void __printf(4, 0) do_test(int bufsize, const char *expect, int elen, const char *fmt, va_list ap) { va_list aq; int ret, written;
- total_tests++; - memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE); va_copy(aq, ap); ret = vsnprintf(test_buffer, bufsize, fmt, aq); va_end(aq);
if (ret != elen) { - pr_warn("vsnprintf(buf, %d, "%s", ...) returned %d, expected %d\n", + tc_fail("vsnprintf(buf, %d, "%s", ...) returned %d, expected %d\n", bufsize, fmt, ret, elen); - return 1; + return; }
if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote before buffer\n", bufsize, fmt); - return 1; + tc_fail("vsnprintf(buf, %d, "%s", ...) wrote before buffer\n", bufsize, fmt); + return; }
if (!bufsize) { if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) { - pr_warn("vsnprintf(buf, 0, "%s", ...) wrote to buffer\n", + tc_fail("vsnprintf(buf, 0, "%s", ...) wrote to buffer\n", fmt); - return 1; } - return 0; + return; }
written = min(bufsize-1, elen); if (test_buffer[written]) { - pr_warn("vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer\n", + tc_fail("vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer\n", bufsize, fmt); - return 1; + return; }
if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator\n", + tc_fail("vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator\n", bufsize, fmt); - return 1; + return; }
if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote beyond buffer\n", bufsize, fmt); - return 1; + tc_fail("vsnprintf(buf, %d, "%s", ...) wrote beyond buffer\n", bufsize, fmt); + return; }
if (memcmp(test_buffer, expect, written)) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'\n", + tc_fail("vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'\n", bufsize, fmt, test_buffer, written, expect); - return 1; + return; } - return 0; }
-static void __printf(3, 4) __init +static void __printf(3, 4) __test(const char *expect, int elen, const char *fmt, ...) { va_list ap; @@ -110,9 +107,8 @@ __test(const char *expect, int elen, const char *fmt, ...) char *p;
if (elen >= BUF_SIZE) { - pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n", - elen, fmt); - failed_tests++; + tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n", + elen, fmt); return; }
@@ -124,19 +120,17 @@ __test(const char *expect, int elen, const char *fmt, ...) * enough and 0), and then we also test that kvasprintf would * be able to print it as expected. */ - failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap); + do_test(BUF_SIZE, expect, elen, fmt, ap); rand = get_random_u32_inclusive(1, elen + 1); /* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */ - failed_tests += do_test(rand, expect, elen, fmt, ap); - failed_tests += do_test(0, expect, elen, fmt, ap); + do_test(rand, expect, elen, fmt, ap); + do_test(0, expect, elen, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap); if (p) { - total_tests++; if (memcmp(p, expect, elen+1)) { - pr_warn("kvasprintf(..., "%s", ...) returned '%s', expected '%s'\n", + tc_fail("kvasprintf(..., "%s", ...) returned '%s', expected '%s'\n", fmt, p, expect); - failed_tests++; } kfree(p); } @@ -146,7 +140,7 @@ __test(const char *expect, int elen, const char *fmt, ...) #define test(expect, fmt, ...) \ __test(expect, strlen(expect), fmt, ##__VA_ARGS__)
-static void __init +static void test_basic(void) { /* Work around annoying "warning: zero-length gnu_printf format string". */ @@ -158,7 +152,7 @@ test_basic(void) __test("xxx\0yyy", 7, "xxx%cyyy", '\0'); }
-static void __init +static void test_number(void) { test("0x1234abcd ", "%#-12x", 0x1234abcd); @@ -180,7 +174,7 @@ test_number(void) test("00|0|0|0|0", "%.2d|%.1d|%.0d|%.*d|%1.0d", 0, 0, 0, 0, 0, 0); }
-static void __init +static void test_string(void) { test("", "%s%.0s", "", "123"); @@ -218,7 +212,7 @@ test_string(void) #define ZEROS "00000000" /* hex 32 zero bits */ #define ONES "ffffffff" /* hex 32 one bits */
-static int __init +static int plain_format(void) { char buf[PLAIN_BUF_SIZE]; @@ -250,7 +244,7 @@ plain_format(void) #define ZEROS "" #define ONES ""
-static int __init +static int plain_format(void) { /* Format is implicitly tested for 32 bit machines by plain_hash() */ @@ -259,7 +253,7 @@ plain_format(void)
#endif /* BITS_PER_LONG == 64 */
-static int __init +static int plain_hash_to_buffer(const void *p, char *buf, size_t len) { int nchars; @@ -278,7 +272,7 @@ plain_hash_to_buffer(const void *p, char *buf, size_t len) return 0; }
-static int __init +static int plain_hash(void) { char buf[PLAIN_BUF_SIZE]; @@ -298,32 +292,29 @@ plain_hash(void) * We can't use test() to test %p because we don't know what output to expect * after an address is hashed. */ -static void __init +static void plain(void) { int err;
if (no_hash_pointers) { pr_warn("skipping plain 'p' tests"); - skipped_tests += 2; return; }
err = plain_hash(); if (err) { - pr_warn("plain 'p' does not appear to be hashed\n"); - failed_tests++; + tc_fail("plain 'p' does not appear to be hashed\n"); return; }
err = plain_format(); if (err) { - pr_warn("hashing plain 'p' has unexpected format\n"); - failed_tests++; + tc_fail("hashing plain 'p' has unexpected format\n"); } }
-static void __init +static void test_hashed(const char *fmt, const void *p) { char buf[PLAIN_BUF_SIZE]; @@ -343,7 +334,7 @@ test_hashed(const char *fmt, const void *p) /* * NULL pointers aren't hashed. */ -static void __init +static void null_pointer(void) { test(ZEROS "00000000", "%p", NULL); @@ -354,7 +345,7 @@ null_pointer(void) /* * Error pointers aren't hashed. */ -static void __init +static void error_pointer(void) { test(ONES "fffffff5", "%p", ERR_PTR(-11)); @@ -364,7 +355,7 @@ error_pointer(void)
#define PTR_INVALID ((void *)0x000000ab)
-static void __init +static void invalid_pointer(void) { test_hashed("%p", PTR_INVALID); @@ -372,18 +363,18 @@ invalid_pointer(void) test("(efault)", "%pE", PTR_INVALID); }
-static void __init +static void symbol_ptr(void) { }
-static void __init +static void kernel_ptr(void) { /* We can't test this without access to kptr_restrict. */ }
-static void __init +static void struct_resource(void) { struct resource test_resource = { @@ -432,7 +423,7 @@ struct_resource(void) "%pR", &test_resource); }
-static void __init +static void struct_range(void) { struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11, @@ -448,17 +439,17 @@ struct_range(void) "%pra", &test_range); }
-static void __init +static void addr(void) { }
-static void __init +static void escaped_str(void) { }
-static void __init +static void hex_string(void) { const char buf[3] = {0xc0, 0xff, 0xee}; @@ -469,7 +460,7 @@ hex_string(void) "%*ph|%*phC|%*phD|%*phN", 3, buf, 3, buf, 3, buf, 3, buf); }
-static void __init +static void mac(void) { const u8 addr[6] = {0x2d, 0x48, 0xd6, 0xfc, 0x7a, 0x05}; @@ -481,7 +472,7 @@ mac(void) test("057afcd6482d", "%pmR", addr); }
-static void __init +static void ip4(void) { struct sockaddr_in sa; @@ -496,19 +487,19 @@ ip4(void) test("001.002.003.004:12345|1.2.3.4:12345", "%piSp|%pISp", &sa, &sa); }
-static void __init +static void ip6(void) { }
-static void __init +static void ip(void) { ip4(); ip6(); }
-static void __init +static void uuid(void) { const char uuid[16] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, @@ -520,7 +511,7 @@ uuid(void) test("03020100-0504-0706-0809-0A0B0C0D0E0F", "%pUL", uuid); }
-static struct dentry test_dentry[4] __initdata = { +static struct dentry test_dentry[4] = { { .d_parent = &test_dentry[0], .d_name = QSTR_INIT(test_dentry[0].d_iname, 3), .d_iname = "foo" }, @@ -535,7 +526,7 @@ static struct dentry test_dentry[4] __initdata = { .d_iname = "romeo" }, };
-static void __init +static void dentry(void) { test("foo", "%pd", &test_dentry[0]); @@ -556,12 +547,12 @@ dentry(void) test(" bravo/alfa| bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]); }
-static void __init +static void struct_va_format(void) { }
-static void __init +static void time_and_date(void) { /* 1543210543 */ @@ -595,12 +586,12 @@ time_and_date(void) test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t); }
-static void __init +static void struct_clk(void) { }
-static void __init +static void large_bitmap(void) { const int nbits = 1 << 16; @@ -614,7 +605,7 @@ large_bitmap(void) bitmap_free(bits); }
-static void __init +static void bitmap(void) { DECLARE_BITMAP(bits, 20); @@ -637,7 +628,7 @@ bitmap(void) large_bitmap(); }
-static void __init +static void netdev_features(void) { } @@ -663,7 +654,7 @@ static const struct page_flags_test pft[] = { "%#x", "kasantag"}, };
-static void __init +static void page_flags_test(int section, int node, int zone, int last_cpupid, int kasan_tag, unsigned long flags, const char *name, char *cmp_buf) @@ -701,7 +692,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid, test(cmp_buf, "%pGp", &flags); }
-static void __init +static void flags(void) { unsigned long flags; @@ -749,7 +740,7 @@ flags(void) kfree(cmp_buffer); }
-static void __init fwnode_pointer(void) +static void fwnode_pointer(void) { const struct software_node first = { .name = "first" }; const struct software_node second = { .name = "second", .parent = &first }; @@ -776,7 +767,7 @@ static void __init fwnode_pointer(void) software_node_unregister_node_group(group); }
-static void __init fourcc_pointer(void) +static void fourcc_pointer(void) { struct { u32 code; @@ -793,7 +784,7 @@ static void __init fourcc_pointer(void) test(try[i].str, "%p4cc", &try[i].code); }
-static void __init +static void errptr(void) { test("-1234", "%pe", ERR_PTR(-1234)); @@ -813,7 +804,7 @@ errptr(void) #endif }
-static void __init +static void test_pointer(void) { plain(); @@ -842,13 +833,15 @@ test_pointer(void) fourcc_pointer(); }
-static void __init selftest(void) +static void printf_test(struct kunit *test) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) return; test_buffer = alloced_buffer + PAD_SIZE;
+ kunittest = test; + test_basic(); test_number(); test_string(); @@ -857,7 +850,18 @@ static void __init selftest(void) kfree(alloced_buffer); }
-KSTM_MODULE_LOADERS(test_printf); +static struct kunit_case printf_test_cases[] = { + KUNIT_CASE(printf_test), + {} +}; + +static struct kunit_suite printf_test_suite = { + .name = "printf", + .test_cases = printf_test_cases, +}; + +kunit_test_suite(printf_test_suite); + MODULE_AUTHOR("Rasmus Villemoes linux@rasmusvillemoes.dk"); MODULE_DESCRIPTION("Test cases for printf facility"); MODULE_LICENSE("GPL"); diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config index dc15aba8d0a3..0a63594177c2 100644 --- a/tools/testing/selftests/lib/config +++ b/tools/testing/selftests/lib/config @@ -1,4 +1,3 @@ -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_PRIME_NUMBERS=m diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh deleted file mode 100755 index 05f4544e87f9..000000000000 --- a/tools/testing/selftests/lib/printf.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 -# Tests the printf infrastructure using test_printf kernel module. -$(dirname $0)/../kselftest/module.sh "printf" test_printf
On Fri, Feb 7, 2025 at 6:30 AM Tamir Duberstein tamird@gmail.com wrote:
Convert the printf() 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.
Apologies for the version churn, I'll have to send one more which removes spurious trailing newlines (not necessary in kunit) and converts logging macros like pr_info to kunit_info.
I'll wait a few days in case folks have other comments.
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
Convert the printf() 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.
Acked-by: Geert Uytterhoeven geert@linux-m68k.org # m68k Signed-off-by: Tamir Duberstein tamird@gmail.com
Documentation/core-api/printk-formats.rst | 2 +- MAINTAINERS | 2 +- arch/m68k/configs/amiga_defconfig | 1 - arch/m68k/configs/apollo_defconfig | 1 - arch/m68k/configs/atari_defconfig | 1 - arch/m68k/configs/bvme6000_defconfig | 1 - arch/m68k/configs/hp300_defconfig | 1 - arch/m68k/configs/mac_defconfig | 1 - arch/m68k/configs/multi_defconfig | 1 - arch/m68k/configs/mvme147_defconfig | 1 - arch/m68k/configs/mvme16x_defconfig | 1 - arch/m68k/configs/q40_defconfig | 1 - arch/m68k/configs/sun3_defconfig | 1 - arch/m68k/configs/sun3x_defconfig | 1 - arch/powerpc/configs/ppc64_defconfig | 1 - lib/Kconfig.debug | 20 +++- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 164 +++++++++++++++--------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 20 files changed, 104 insertions(+), 104 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index ecccc0473da9..0d9461bd6964 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -661,7 +661,7 @@ Do *not* use it from C. Thanks ====== -If you add other %p extensions, please extend <lib/test_printf.c> with +If you add other %p extensions, please extend <lib/printf_kunit.c> with one or more test cases, if at all feasible. Thank you for your cooperation and attention. diff --git a/MAINTAINERS b/MAINTAINERS index 896a307fa065..57e366d356bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25368,7 +25368,7 @@ 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/test_printf.c +F: lib/printf_kunit.c F: lib/test_scanf.c F: lib/vsprintf.c diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig index dbf2ea561c85..e8c5e08fb6ec 100644 --- a/arch/m68k/configs/amiga_defconfig +++ b/arch/m68k/configs/amiga_defconfig @@ -622,7 +622,6 @@ CONFIG_ATOMIC64_SELFTEST=m CONFIG_ASYNC_RAID6_TEST=m CONFIG_TEST_HEXDUMP=m CONFIG_TEST_KSTRTOX=m -CONFIG_TEST_PRINTF=m CONFIG_TEST_SCANF=m CONFIG_TEST_BITMAP=m CONFIG_TEST_UUID=m
If/when you do re-roll a v3, can you split the defconfig changes off to a separate patch? It's a little annoying to scroll through all those mechanical one-liner diffs to get to the actual changes.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1af972a92d06..4834cac1eb8f 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST config TEST_HEXDUMP tristate "Test functions located in the hexdump module at runtime" +config PRINTF_KUNIT_TEST
- tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTS
- depends on KUNIT
- default KUNIT_ALL_TESTS
- help
Enable this option to test the printf functions at boot.
Well, not necessarily at boot unless built-in?
KUnit tests run during boot and output the results to the debug login TAP format (http://testanything.org/). Only useful for kernel devsrunning the KUnit test harness, and not intended for inclusion into aproduction build.For more information on KUnit and unit tests in general please referto the KUnit documentation in Documentation/dev-tools/kunit/.If unsure, say N.
I see that this is copy-pasted from other kunit config items, but not all of them have all this boilerplate, and I don't think it's useful (and, the mentions of "at boot" and "during boot" are actively misleading). Other kunit config items are shorter and more to the point, e.g.
Enable this to turn on 'list_sort()' function test. This test is executed only once during system boot (so affects only boot time), or at module load time.
If unsure, say N.
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/test_printf.c b/lib/printf_kunit.c similarity index 89% rename from lib/test_printf.c rename to lib/printf_kunit.c index 59dbe4f9a4cb..fe6f4df92dd5 100644 --- a/lib/test_printf.c +++ b/lib/printf_kunit.c @@ -5,7 +5,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> +#include <kunit/test.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/printk.h> @@ -25,8 +25,6 @@ #include <linux/property.h> -#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,72 +35,71 @@ block \ __diag_pop(); -KSTM_MODULE_GLOBALS(); +static char *test_buffer; +static char *alloced_buffer;
+static struct kunit *kunittest; -static char *test_buffer __initdata; -static char *alloced_buffer __initdata; +#define tc_fail(fmt, ...) \
- KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
-static int __printf(4, 0) __init +static void __printf(4, 0) do_test(int bufsize, const char *expect, int elen, const char *fmt, va_list ap) { va_list aq; int ret, written;
- total_tests++;
- memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE); va_copy(aq, ap); ret = vsnprintf(test_buffer, bufsize, fmt, aq); va_end(aq);
if (ret != elen) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n", bufsize, fmt, ret, elen);
return 1;
return;if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);return 1;
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);return;if (!bufsize) { if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
tc_fail("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n", fmt);
return 1;return 0;
return;written = min(bufsize-1, elen); if (test_buffer[written]) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n", bufsize, fmt);
return 1;
return;if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n", bufsize, fmt);
return 1;
return;if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);return 1;
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);return;if (memcmp(test_buffer, expect, written)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n", bufsize, fmt, test_buffer, written, expect);
return 1;
return;
- return 0;
} -static void __printf(3, 4) __init +static void __printf(3, 4) __test(const char *expect, int elen, const char *fmt, ...) { va_list ap; @@ -110,9 +107,8 @@ __test(const char *expect, int elen, const char *fmt, ...) char *p; if (elen >= BUF_SIZE) {
pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",elen, fmt);failed_tests++;
tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n",elen, fmt);@@ -124,19 +120,17 @@ __test(const char *expect, int elen, const char *fmt, ...) * enough and 0), and then we also test that kvasprintf would * be able to print it as expected. */
- failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
- do_test(BUF_SIZE, expect, elen, fmt, ap); rand = get_random_u32_inclusive(1, elen + 1); /* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
- failed_tests += do_test(rand, expect, elen, fmt, ap);
- failed_tests += do_test(0, expect, elen, fmt, ap);
- do_test(rand, expect, elen, fmt, ap);
- do_test(0, expect, elen, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap); if (p) {
total_tests++;pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
tc_fail("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n", fmt, p, expect);
failed_tests++;
So reading through this, is there really any reason to drop those existing total_tests and failed_tests tallies? Since you do need to keep the control flow the same, leaving the return types and "return 1"/"return 0" and their uses in updating failed_tests would also reduce the patch size quite significantly.
So they no longer come from some KSTM_MODULE_GLOBALS(), and thus need to be explicitly declared in this module, but that's exactly how it was originally until someone decided to lift that from the the printf suite to kstm.
Yes, they'd need to be explicitly initialized to 0 during kunit_init since they are no longer use-once __initdata variable, and some kunit_exit logic would need to "manually" report them.
Rasmus
On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
[...]
If/when you do re-roll a v3, can you split the defconfig changes off to a separate patch? It's a little annoying to scroll through all those mechanical one-liner diffs to get to the actual changes.
Yep. I'll split them into one for m68k and another for powerpc. Geert, I'll move your Acked-by to the m68k patch.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1af972a92d06..4834cac1eb8f 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST config TEST_HEXDUMP tristate "Test functions located in the hexdump module at runtime"
+config PRINTF_KUNIT_TEST
tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTSdepends on KUNITdefault KUNIT_ALL_TESTShelpEnable this option to test the printf functions at boot.Well, not necessarily at boot unless built-in?
KUnit tests run during boot and output the results to the debug login TAP format (http://testanything.org/). Only useful for kernel devsrunning the KUnit test harness, and not intended for inclusion into aproduction build.For more information on KUnit and unit tests in general please referto the KUnit documentation in Documentation/dev-tools/kunit/.If unsure, say N.I see that this is copy-pasted from other kunit config items, but not all of them have all this boilerplate, and I don't think it's useful (and, the mentions of "at boot" and "during boot" are actively misleading). Other kunit config items are shorter and more to the point, e.g.
Enable this to turn on 'list_sort()' function test. This test is executed only once during system boot (so affects only boot time), or at module load time. If unsure, say N.
Very good point. Truthfully this boilerplate is a result of checkpatch.pl nagging about this paragraph being a certain length. I'll drop it and just write "Enable this option to test the printf functions at runtime.".
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/test_printf.c b/lib/printf_kunit.c similarity index 89% rename from lib/test_printf.c rename to lib/printf_kunit.c index 59dbe4f9a4cb..fe6f4df92dd5 100644 --- a/lib/test_printf.c +++ b/lib/printf_kunit.c @@ -5,7 +5,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/init.h> +#include <kunit/test.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/printk.h> @@ -25,8 +25,6 @@
#include <linux/property.h>
-#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,72 +35,71 @@ block \ __diag_pop();
-KSTM_MODULE_GLOBALS(); +static char *test_buffer; +static char *alloced_buffer;
+static struct kunit *kunittest;
-static char *test_buffer __initdata; -static char *alloced_buffer __initdata; +#define tc_fail(fmt, ...) \
KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)-static int __printf(4, 0) __init +static void __printf(4, 0) do_test(int bufsize, const char *expect, int elen, const char *fmt, va_list ap) { va_list aq; int ret, written;
total_tests++;memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE); va_copy(aq, ap); ret = vsnprintf(test_buffer, bufsize, fmt, aq); va_end(aq); if (ret != elen) {pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n", bufsize, fmt, ret, elen);
return 1;
return; } if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);return 1;
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);return; } if (!bufsize) { if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
tc_fail("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n", fmt);
return 1; }return 0;
return; } written = min(bufsize-1, elen); if (test_buffer[written]) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n", bufsize, fmt);
return 1;
return; } if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n", bufsize, fmt);
return 1;
return; } if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);return 1;
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);return; } if (memcmp(test_buffer, expect, written)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n", bufsize, fmt, test_buffer, written, expect);
return 1;
return; }
return 0;}
-static void __printf(3, 4) __init +static void __printf(3, 4) __test(const char *expect, int elen, const char *fmt, ...) { va_list ap; @@ -110,9 +107,8 @@ __test(const char *expect, int elen, const char *fmt, ...) char *p;
if (elen >= BUF_SIZE) {
pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",elen, fmt);failed_tests++;
tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n",elen, fmt); return; }@@ -124,19 +120,17 @@ __test(const char *expect, int elen, const char *fmt, ...) * enough and 0), and then we also test that kvasprintf would * be able to print it as expected. */
failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
do_test(BUF_SIZE, expect, elen, fmt, ap); rand = get_random_u32_inclusive(1, elen + 1); /* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
failed_tests += do_test(rand, expect, elen, fmt, ap);failed_tests += do_test(0, expect, elen, fmt, ap);
do_test(rand, expect, elen, fmt, ap);do_test(0, expect, elen, fmt, ap); p = kvasprintf(GFP_KERNEL, fmt, ap); if (p) {
total_tests++; if (memcmp(p, expect, elen+1)) {pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
tc_fail("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n", fmt, p, expect);
failed_tests++; } kfree(p); }So reading through this, is there really any reason to drop those existing total_tests and failed_tests tallies? Since you do need to keep the control flow the same, leaving the return types and "return 1"/"return 0" and their uses in updating failed_tests would also reduce the patch size quite significantly.
So they no longer come from some KSTM_MODULE_GLOBALS(), and thus need to be explicitly declared in this module, but that's exactly how it was originally until someone decided to lift that from the the printf suite to kstm.
Yes, they'd need to be explicitly initialized to 0 during kunit_init since they are no longer use-once __initdata variable, and some kunit_exit logic would need to "manually" report them.
If I understand your concern correctly, you only really care about `total_tests`, right? I think it's slightly unidiomatic to count tests outside the framework this way but it's not a hill I'll die on.
Just to elaborate on why I think this unidiomatic: the KUnit test runner script that produces human-readable output doesn't emit logs unless something fails. In the success case, you'd get (before the next patch that breaks this into many tests):
[09:34:15] ==================== printf (1 subtest) ==================== [09:34:15] [PASSED] printf_test [09:34:15] ===================== [PASSED] printf ====================== [09:34:15] ============================================================ [09:34:15] Testing complete. Ran 1 tests: passed: 1
but the raw output does contain the tally:
KTAP version 1 1..1 KTAP version 1 # Subtest: printf # module: printf_kunit 1..1 ok 1 printf_test # printf: ran 448 tests ok 1 printf
So assuming you're OK with this compromise, I'll go ahead and spin v3.
Thanks for the review!
Hi Tamir,
On Mon, 10 Feb 2025 at 15:37, Tamir Duberstein tamird@gmail.com wrote:
On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
[...]
If/when you do re-roll a v3, can you split the defconfig changes off to a separate patch? It's a little annoying to scroll through all those mechanical one-liner diffs to get to the actual changes.
Yep. I'll split them into one for m68k and another for powerpc. Geert, I'll move your Acked-by to the m68k patch.
Fine for me!
Alternatively, you could just drop them. I do refresh the m68k defconfigs after every kernel release, but these updates only go upstream one cycle later. In this case that doesn't matter at all.
Gr{oetje,eeting}s,
Geert
On Mon, Feb 10, 2025 at 9:52 AM Geert Uytterhoeven geert@linux-m68k.org wrote:
Hi Tamir,
On Mon, 10 Feb 2025 at 15:37, Tamir Duberstein tamird@gmail.com wrote:
On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
[...]
If/when you do re-roll a v3, can you split the defconfig changes off to a separate patch? It's a little annoying to scroll through all those mechanical one-liner diffs to get to the actual changes.
Yep. I'll split them into one for m68k and another for powerpc. Geert, I'll move your Acked-by to the m68k patch.
Fine for me!
Alternatively, you could just drop them. I do refresh the m68k defconfigs after every kernel release, but these updates only go upstream one cycle later. In this case that doesn't matter at all.
Will do, and in scanf and bitmap as well.
Cheers. Tamir
Use `suite_{init,exit}` and move all tests into `printf_test_cases`. This gives us nicer output in the event of a failure.
Combine `plain_format` and `plain_hash` into `hash_pointer` since they're testing the same scenario.
Signed-off-by: Tamir Duberstein tamird@gmail.com --- lib/printf_kunit.c | 286 +++++++++++++++++++---------------------------------- 1 file changed, 103 insertions(+), 183 deletions(-)
diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c index fe6f4df92dd5..b4b6f9943025 100644 --- a/lib/printf_kunit.c +++ b/lib/printf_kunit.c @@ -38,13 +38,11 @@ static char *test_buffer; static char *alloced_buffer;
-static struct kunit *kunittest; - #define tc_fail(fmt, ...) \ KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
-static void __printf(4, 0) -do_test(int bufsize, const char *expect, int elen, +static void __printf(5, 0) +do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen, const char *fmt, va_list ap) { va_list aq; @@ -99,8 +97,8 @@ do_test(int bufsize, const char *expect, int elen, } }
-static void __printf(3, 4) -__test(const char *expect, int elen, const char *fmt, ...) +static void __printf(4, 5) +__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, ...) { va_list ap; int rand; @@ -120,11 +118,11 @@ __test(const char *expect, int elen, const char *fmt, ...) * enough and 0), and then we also test that kvasprintf would * be able to print it as expected. */ - do_test(BUF_SIZE, expect, elen, fmt, ap); + do_test(kunittest, BUF_SIZE, expect, elen, fmt, ap); rand = get_random_u32_inclusive(1, elen + 1); /* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */ - do_test(rand, expect, elen, fmt, ap); - do_test(0, expect, elen, fmt, ap); + do_test(kunittest, rand, expect, elen, fmt, ap); + do_test(kunittest, 0, expect, elen, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap); if (p) { @@ -138,10 +136,10 @@ __test(const char *expect, int elen, const char *fmt, ...) }
#define test(expect, fmt, ...) \ - __test(expect, strlen(expect), fmt, ##__VA_ARGS__) + __test(kunittest, expect, strlen(expect), fmt, ##__VA_ARGS__)
static void -test_basic(void) +test_basic(struct kunit *kunittest) { /* Work around annoying "warning: zero-length gnu_printf format string". */ char nul = '\0'; @@ -149,11 +147,11 @@ test_basic(void) test("", &nul); test("100%", "100%%"); test("xxx%yyy", "xxx%cyyy", '%'); - __test("xxx\0yyy", 7, "xxx%cyyy", '\0'); + __test(kunittest, "xxx\0yyy", 7, "xxx%cyyy", '\0'); }
static void -test_number(void) +test_number(struct kunit *kunittest) { test("0x1234abcd ", "%#-12x", 0x1234abcd); test(" 0x1234abcd", "%#12x", 0x1234abcd); @@ -175,7 +173,7 @@ test_number(void) }
static void -test_string(void) +test_string(struct kunit *kunittest) { test("", "%s%.0s", "", "123"); test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456"); @@ -212,29 +210,6 @@ test_string(void) #define ZEROS "00000000" /* hex 32 zero bits */ #define ONES "ffffffff" /* hex 32 one bits */
-static int -plain_format(void) -{ - char buf[PLAIN_BUF_SIZE]; - int nchars; - - nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR); - - if (nchars != PTR_WIDTH) - return -1; - - if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { - pr_warn("crng possibly not yet initialized. plain 'p' buffer contains "%s"", - PTR_VAL_NO_CRNG); - return 0; - } - - if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0) - return -1; - - return 0; -} - #else
#define PTR_WIDTH 8 @@ -244,89 +219,44 @@ plain_format(void) #define ZEROS "" #define ONES ""
-static int -plain_format(void) -{ - /* Format is implicitly tested for 32 bit machines by plain_hash() */ - return 0; -} - #endif /* BITS_PER_LONG == 64 */
-static int -plain_hash_to_buffer(const void *p, char *buf, size_t len) +static void +plain_hash_to_buffer(struct kunit *kunittest, const void *p, char *buf, size_t len) { - int nchars; - - nchars = snprintf(buf, len, "%p", p); - - if (nchars != PTR_WIDTH) - return -1; + KUNIT_ASSERT_EQ(kunittest, snprintf(buf, len, "%p", p), PTR_WIDTH);
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { pr_warn("crng possibly not yet initialized. plain 'p' buffer contains "%s"", PTR_VAL_NO_CRNG); - return 0; } - - return 0; -} - -static int -plain_hash(void) -{ - char buf[PLAIN_BUF_SIZE]; - int ret; - - ret = plain_hash_to_buffer(PTR, buf, PLAIN_BUF_SIZE); - if (ret) - return ret; - - if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0) - return -1; - - return 0; }
-/* - * We can't use test() to test %p because we don't know what output to expect - * after an address is hashed. - */ static void -plain(void) +hash_pointer(struct kunit *kunittest) { - int err; + if (no_hash_pointers) + kunit_skip(kunittest, "hash pointers disabled");
- if (no_hash_pointers) { - pr_warn("skipping plain 'p' tests"); - return; - } + char buf[PLAIN_BUF_SIZE];
- err = plain_hash(); - if (err) { - tc_fail("plain 'p' does not appear to be hashed\n"); - return; - } + plain_hash_to_buffer(kunittest, PTR, buf, PLAIN_BUF_SIZE);
- err = plain_format(); - if (err) { - tc_fail("hashing plain 'p' has unexpected format\n"); - } + /* + * We can't use test() to test %p because we don't know what output to expect + * after an address is hashed. + */ + + KUNIT_EXPECT_MEMEQ(kunittest, buf, ZEROS, strlen(ZEROS)); + KUNIT_EXPECT_MEMNEQ(kunittest, buf+strlen(ZEROS), PTR_STR, PTR_WIDTH); }
static void -test_hashed(const char *fmt, const void *p) +test_hashed(struct kunit *kunittest, const char *fmt, const void *p) { char buf[PLAIN_BUF_SIZE]; - int ret;
- /* - * No need to increase failed test counter since this is assumed - * to be called after plain(). - */ - ret = plain_hash_to_buffer(p, buf, PLAIN_BUF_SIZE); - if (ret) - return; + plain_hash_to_buffer(kunittest, p, buf, PLAIN_BUF_SIZE);
test(buf, fmt, p); } @@ -335,7 +265,7 @@ test_hashed(const char *fmt, const void *p) * NULL pointers aren't hashed. */ static void -null_pointer(void) +null_pointer(struct kunit *kunittest) { test(ZEROS "00000000", "%p", NULL); test(ZEROS "00000000", "%px", NULL); @@ -346,7 +276,7 @@ null_pointer(void) * Error pointers aren't hashed. */ static void -error_pointer(void) +error_pointer(struct kunit *kunittest) { test(ONES "fffffff5", "%p", ERR_PTR(-11)); test(ONES "fffffff5", "%px", ERR_PTR(-11)); @@ -356,26 +286,26 @@ error_pointer(void) #define PTR_INVALID ((void *)0x000000ab)
static void -invalid_pointer(void) +invalid_pointer(struct kunit *kunittest) { - test_hashed("%p", PTR_INVALID); + test_hashed(kunittest, "%p", PTR_INVALID); test(ZEROS "000000ab", "%px", PTR_INVALID); test("(efault)", "%pE", PTR_INVALID); }
static void -symbol_ptr(void) +symbol_ptr(struct kunit *kunittest) { }
static void -kernel_ptr(void) +kernel_ptr(struct kunit *kunittest) { /* We can't test this without access to kptr_restrict. */ }
static void -struct_resource(void) +struct_resource(struct kunit *kunittest) { struct resource test_resource = { .start = 0xc0ffee00, @@ -424,7 +354,7 @@ struct_resource(void) }
static void -struct_range(void) +struct_range(struct kunit *kunittest) { struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11, 0xc0ffee00ba5eba11); @@ -440,17 +370,17 @@ struct_range(void) }
static void -addr(void) +addr(struct kunit *kunittest) { }
static void -escaped_str(void) +escaped_str(struct kunit *kunittest) { }
static void -hex_string(void) +hex_string(struct kunit *kunittest) { const char buf[3] = {0xc0, 0xff, 0xee};
@@ -461,7 +391,7 @@ hex_string(void) }
static void -mac(void) +mac(struct kunit *kunittest) { const u8 addr[6] = {0x2d, 0x48, 0xd6, 0xfc, 0x7a, 0x05};
@@ -473,7 +403,7 @@ mac(void) }
static void -ip4(void) +ip4(struct kunit *kunittest) { struct sockaddr_in sa;
@@ -488,19 +418,12 @@ ip4(void) }
static void -ip6(void) +ip6(struct kunit *kunittest) { }
static void -ip(void) -{ - ip4(); - ip6(); -} - -static void -uuid(void) +uuid(struct kunit *kunittest) { const char uuid[16] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}; @@ -527,7 +450,7 @@ static struct dentry test_dentry[4] = { };
static void -dentry(void) +dentry(struct kunit *kunittest) { test("foo", "%pd", &test_dentry[0]); test("foo", "%pd2", &test_dentry[0]); @@ -548,12 +471,12 @@ dentry(void) }
static void -struct_va_format(void) +struct_va_format(struct kunit *kunittest) { }
static void -time_and_date(void) +time_and_date(struct kunit *kunittest) { /* 1543210543 */ const struct rtc_time tm = { @@ -587,12 +510,12 @@ time_and_date(void) }
static void -struct_clk(void) +struct_clk(struct kunit *kunittest) { }
static void -large_bitmap(void) +large_bitmap(struct kunit *kunittest) { const int nbits = 1 << 16; unsigned long *bits = bitmap_zalloc(nbits, GFP_KERNEL); @@ -606,7 +529,7 @@ large_bitmap(void) }
static void -bitmap(void) +bitmap(struct kunit *kunittest) { DECLARE_BITMAP(bits, 20); const int primes[] = {2,3,5,7,11,13,17,19}; @@ -625,11 +548,11 @@ bitmap(void) test("fffff|fffff", "%20pb|%*pb", bits, 20, bits); test("0-19|0-19", "%20pbl|%*pbl", bits, 20, bits);
- large_bitmap(); + large_bitmap(kunittest); }
static void -netdev_features(void) +netdev_features(struct kunit *kunittest) { }
@@ -655,8 +578,8 @@ static const struct page_flags_test pft[] = { };
static void -page_flags_test(int section, int node, int zone, int last_cpupid, - int kasan_tag, unsigned long flags, const char *name, +page_flags_test(struct kunit *kunittest, int section, int node, int zone, + int last_cpupid, int kasan_tag, unsigned long flags, const char *name, char *cmp_buf) { unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; @@ -693,7 +616,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid, }
static void -flags(void) +flags(struct kunit *kunittest) { unsigned long flags; char *cmp_buffer; @@ -704,14 +627,14 @@ flags(void) return;
flags = 0; - page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer); + page_flags_test(kunittest, 0, 0, 0, 0, 0, flags, "", cmp_buffer);
flags = 1UL << NR_PAGEFLAGS; - page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer); + page_flags_test(kunittest, 0, 0, 0, 0, 0, flags, "", cmp_buffer);
flags |= 1UL << PG_uptodate | 1UL << PG_dirty | 1UL << PG_lru | 1UL << PG_active | 1UL << PG_swapbacked; - page_flags_test(1, 1, 1, 0x1fffff, 1, flags, + page_flags_test(kunittest, 1, 1, 1, 0x1fffff, 1, flags, "uptodate|dirty|lru|active|swapbacked", cmp_buffer);
@@ -740,7 +663,7 @@ flags(void) kfree(cmp_buffer); }
-static void fwnode_pointer(void) +static void fwnode_pointer(struct kunit *kunittest) { const struct software_node first = { .name = "first" }; const struct software_node second = { .name = "second", .parent = &first }; @@ -754,8 +677,7 @@ static void fwnode_pointer(void)
rval = software_node_register_node_group(group); if (rval) { - pr_warn("cannot register softnodes; rval %d\n", rval); - return; + kunit_skip(kunittest, "cannot register softnodes; rval %d\n", rval); }
test(full_name_second, "%pfw", software_node_fwnode(&second)); @@ -767,7 +689,7 @@ static void fwnode_pointer(void) software_node_unregister_node_group(group); }
-static void fourcc_pointer(void) +static void fourcc_pointer(struct kunit *kunittest) { struct { u32 code; @@ -785,13 +707,13 @@ static void fourcc_pointer(void) }
static void -errptr(void) +errptr(struct kunit *kunittest) { test("-1234", "%pe", ERR_PTR(-1234));
/* Check that %pe with a non-ERR_PTR gets treated as ordinary %p. */ BUILD_BUG_ON(IS_ERR(PTR)); - test_hashed("%pe", PTR); + test_hashed(kunittest, "%pe", PTR);
#ifdef CONFIG_SYMBOLIC_ERRNAME test("(-ENOTSOCK)", "(%pe)", ERR_PTR(-ENOTSOCK)); @@ -804,59 +726,57 @@ errptr(void) #endif }
-static void -test_pointer(void) -{ - plain(); - null_pointer(); - error_pointer(); - invalid_pointer(); - symbol_ptr(); - kernel_ptr(); - struct_resource(); - struct_range(); - addr(); - escaped_str(); - hex_string(); - mac(); - ip(); - uuid(); - dentry(); - struct_va_format(); - time_and_date(); - struct_clk(); - bitmap(); - netdev_features(); - flags(); - errptr(); - fwnode_pointer(); - fourcc_pointer(); -} - -static void printf_test(struct kunit *test) +static struct kunit_case printf_test_cases[] = { + KUNIT_CASE(test_basic), + KUNIT_CASE(test_number), + KUNIT_CASE(test_string), + KUNIT_CASE(hash_pointer), + KUNIT_CASE(null_pointer), + KUNIT_CASE(error_pointer), + KUNIT_CASE(invalid_pointer), + KUNIT_CASE(symbol_ptr), + KUNIT_CASE(kernel_ptr), + KUNIT_CASE(struct_resource), + KUNIT_CASE(struct_range), + KUNIT_CASE(addr), + KUNIT_CASE(escaped_str), + KUNIT_CASE(hex_string), + KUNIT_CASE(mac), + KUNIT_CASE(ip4), + KUNIT_CASE(ip6), + KUNIT_CASE(uuid), + KUNIT_CASE(dentry), + KUNIT_CASE(struct_va_format), + KUNIT_CASE(time_and_date), + KUNIT_CASE(struct_clk), + KUNIT_CASE(bitmap), + KUNIT_CASE(netdev_features), + KUNIT_CASE(flags), + KUNIT_CASE(errptr), + KUNIT_CASE(fwnode_pointer), + KUNIT_CASE(fourcc_pointer), + {} +}; + +static int printf_suite_init(struct kunit_suite *suite) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) - return; + return -1; test_buffer = alloced_buffer + PAD_SIZE; + return 0; +}
- kunittest = test; - - test_basic(); - test_number(); - test_string(); - test_pointer(); - +static void printf_suite_exit(struct kunit_suite *suite) +{ kfree(alloced_buffer); }
-static struct kunit_case printf_test_cases[] = { - KUNIT_CASE(printf_test), - {} -};
static struct kunit_suite printf_test_suite = { .name = "printf", + .suite_init = printf_suite_init, + .suite_exit = printf_suite_exit, .test_cases = printf_test_cases, };
linux-kselftest-mirror@lists.linaro.org
-
Geert Uytterhoeven -
Rasmus Villemoes -
Tamir Duberstein