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 --- 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/printf_kunit.c | 757 ++++++++++++++++++++++++++ lib/test_printf.c | 863 ------------------------------ tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 21 files changed, 777 insertions(+), 887 deletions(-) --- base-commit: a86bf2283d2c9769205407e2b54777c03d012939 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.
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} | 522 ++++++++++++++---------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 20 files changed, 261 insertions(+), 305 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 53% rename from lib/test_printf.c rename to lib/printf_kunit.c index 59dbe4f9a4cb..e889aca69eba 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,84 +35,53 @@ block \ __diag_pop();
-KSTM_MODULE_GLOBALS(); - -static char *test_buffer __initdata; -static char *alloced_buffer __initdata; +static char *test_buffer; +static char *alloced_buffer;
-static int __printf(4, 0) __init -do_test(int bufsize, const char *expect, int elen, +static void __printf(5, 0) +do_test(struct kunit *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", - bufsize, fmt, ret, elen); - return 1; - } - - if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote before buffer\n", bufsize, fmt); - return 1; - } + KUNIT_EXPECT_EQ_MSG(test, ret, elen, "vsnprintf(buf, %d, "%s", ...)", bufsize, fmt); + KUNIT_EXPECT_FALSE_MSG(test, memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE), + "vsnprintf(buf, %d, "%s", ...) wrote before buffer", bufsize, fmt);
if (!bufsize) { - if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) { - pr_warn("vsnprintf(buf, 0, "%s", ...) wrote to buffer\n", - fmt); - return 1; - } - return 0; + KUNIT_EXPECT_FALSE_MSG(test, + memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE), + "vsnprintf(buf, 0, "%s", ...) wrote to buffer", fmt); + return; }
written = min(bufsize-1, elen); - if (test_buffer[written]) { - pr_warn("vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer\n", - bufsize, fmt); - return 1; - } - - if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator\n", - bufsize, fmt); - return 1; - } - - 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; - } - - if (memcmp(test_buffer, expect, written)) { - pr_warn("vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'\n", - bufsize, fmt, test_buffer, written, expect); - return 1; - } - return 0; + KUNIT_EXPECT_FALSE_MSG(test, test_buffer[written], + "vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer", bufsize, fmt); + KUNIT_EXPECT_FALSE_MSG(test, + memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1)), + "vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator", bufsize, fmt); + KUNIT_EXPECT_FALSE_MSG(test, + memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize), + "vsnprintf(buf, %d, "%s", ...) wrote beyond buffer", bufsize, fmt); + KUNIT_EXPECT_MEMEQ_MSG(test, test_buffer, expect, written, + "vsnprintf(buf, %d, "%s", ...)", bufsize, fmt); }
-static void __printf(3, 4) __init -__test(const char *expect, int elen, const char *fmt, ...) +static void __printf(4, 5) +__test(struct kunit *test, const char *expect, int elen, const char *fmt, ...) { va_list ap; int rand; 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++; - return; - } + KUNIT_EXPECT_LE_MSG(test, elen, BUF_SIZE, "format was '%s'", fmt);
va_start(ap, fmt);
@@ -124,50 +91,45 @@ __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(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(test, rand, expect, elen, fmt, ap); + do_test(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", - fmt, p, expect); - failed_tests++; - } + KUNIT_EXPECT_MEMEQ_MSG(test, p, expect, elen+1, "kvasprintf("%s", ...)", fmt); kfree(p); } va_end(ap); }
-#define test(expect, fmt, ...) \ - __test(expect, strlen(expect), fmt, ##__VA_ARGS__) +#define tc(test, expect, fmt, ...) \ + __test(test, expect, strlen(expect), fmt, ##__VA_ARGS__)
-static void __init -test_basic(void) +static void +test_basic(struct kunit *test) { /* Work around annoying "warning: zero-length gnu_printf format string". */ char nul = '\0';
- test("", &nul); - test("100%", "100%%"); - test("xxx%yyy", "xxx%cyyy", '%'); - __test("xxx\0yyy", 7, "xxx%cyyy", '\0'); + tc(test, "", &nul); + tc(test, "100%", "100%%"); + tc(test, "xxx%yyy", "xxx%cyyy", '%'); + __test(test, "xxx\0yyy", 7, "xxx%cyyy", '\0'); }
-static void __init -test_number(void) +static void +test_number(struct kunit *test) { - test("0x1234abcd ", "%#-12x", 0x1234abcd); - test(" 0x1234abcd", "%#12x", 0x1234abcd); - test("0|001| 12|+123| 1234|-123|-1234", "%d|%03d|%3d|%+d|% d|%+d|% d", 0, 1, 12, 123, 1234, -123, -1234); + tc(test, "0x1234abcd ", "%#-12x", 0x1234abcd); + tc(test, " 0x1234abcd", "%#12x", 0x1234abcd); + tc(test, "0|001| 12|+123| 1234|-123|-1234", "%d|%03d|%3d|%+d|% d|%+d|% d", 0, 1, 12, 123, 1234, -123, -1234); NOWARN(-Wformat, "Intentionally test narrowing conversion specifiers.", { - test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); - test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); - test("2015122420151225", "%ho%ho%#ho", 1037, 5282, -11627); + tc(test, "0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); + tc(test, "0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); + tc(test, "2015122420151225", "%ho%ho%#ho", 1037, 5282, -11627); }) /* * POSIX/C99: »The result of converting zero with an explicit @@ -177,17 +139,17 @@ test_number(void) * case. This test case simply documents the current * behaviour. */ - test("00|0|0|0|0", "%.2d|%.1d|%.0d|%.*d|%1.0d", 0, 0, 0, 0, 0, 0); + tc(test, "00|0|0|0|0", "%.2d|%.1d|%.0d|%.*d|%1.0d", 0, 0, 0, 0, 0, 0); }
-static void __init -test_string(void) +static void +test_string(struct kunit *test) { - test("", "%s%.0s", "", "123"); - test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456"); - test("1 | 2|3 | 4|5 ", "%-3s|%3s|%-*s|%*s|%*s", "1", "2", 3, "3", 3, "4", -3, "5"); - test("1234 ", "%-10.4s", "123456"); - test(" 1234", "%10.4s", "123456"); + tc(test, "", "%s%.0s", "", "123"); + tc(test, "ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456"); + tc(test, "1 | 2|3 | 4|5 ", "%-3s|%3s|%-*s|%*s|%*s", "1", "2", 3, "3", 3, "4", -3, "5"); + tc(test, "1234 ", "%-10.4s", "123456"); + tc(test, " 1234", "%10.4s", "123456"); /* * POSIX and C99 say that a negative precision (which is only * possible to pass via a * argument) should be treated as if @@ -201,10 +163,10 @@ test_string(void) * anyone ever feel the need to follow the standards more * closely, this can be revisited. */ - test(" ", "%4.*s", -5, "123456"); - test("123456", "%.s", "123456"); - test("a||", "%.s|%.0s|%.*s", "a", "b", 0, "c"); - test("a | | ", "%-3.s|%-3.0s|%-3.*s", "a", "b", 0, "c"); + tc(test, " ", "%4.*s", -5, "123456"); + tc(test, "123456", "%.s", "123456"); + tc(test, "a||", "%.s|%.0s|%.*s", "a", "b", 0, "c"); + tc(test, "a | | ", "%-3.s|%-3.0s|%-3.*s", "a", "b", 0, "c"); }
#define PLAIN_BUF_SIZE 64 /* leave some space so we don't oops */ @@ -218,7 +180,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 +212,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 +221,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 +240,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,33 +260,20 @@ 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 -plain(void) +static void +plain(struct kunit *test) { - 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++; return; }
- err = plain_format(); - if (err) { - pr_warn("hashing plain 'p' has unexpected format\n"); - failed_tests++; - } + KUNIT_EXPECT_FALSE(test, plain_hash()); + KUNIT_EXPECT_FALSE(test, plain_format()); }
-static void __init -test_hashed(const char *fmt, const void *p) +static void +test_hashed(struct kunit *test, const char *fmt, const void *p) { char buf[PLAIN_BUF_SIZE]; int ret; @@ -337,54 +286,54 @@ test_hashed(const char *fmt, const void *p) if (ret) return;
- test(buf, fmt, p); + tc(test, buf, fmt, p); }
/* * NULL pointers aren't hashed. */ -static void __init -null_pointer(void) +static void +null_pointer(struct kunit *test) { - test(ZEROS "00000000", "%p", NULL); - test(ZEROS "00000000", "%px", NULL); - test("(null)", "%pE", NULL); + tc(test, ZEROS "00000000", "%p", NULL); + tc(test, ZEROS "00000000", "%px", NULL); + tc(test, "(null)", "%pE", NULL); }
/* * Error pointers aren't hashed. */ -static void __init -error_pointer(void) +static void +error_pointer(struct kunit *test) { - test(ONES "fffffff5", "%p", ERR_PTR(-11)); - test(ONES "fffffff5", "%px", ERR_PTR(-11)); - test("(efault)", "%pE", ERR_PTR(-11)); + tc(test, ONES "fffffff5", "%p", ERR_PTR(-11)); + tc(test, ONES "fffffff5", "%px", ERR_PTR(-11)); + tc(test, "(efault)", "%pE", ERR_PTR(-11)); }
#define PTR_INVALID ((void *)0x000000ab)
-static void __init -invalid_pointer(void) +static void +invalid_pointer(struct kunit *test) { - test_hashed("%p", PTR_INVALID); - test(ZEROS "000000ab", "%px", PTR_INVALID); - test("(efault)", "%pE", PTR_INVALID); + test_hashed(test, "%p", PTR_INVALID); + tc(test, ZEROS "000000ab", "%px", PTR_INVALID); + tc(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 -struct_resource(void) +static void +struct_resource(struct kunit *test) { struct resource test_resource = { .start = 0xc0ffee00, @@ -392,7 +341,7 @@ struct_resource(void) .flags = IORESOURCE_MEM, };
- test("[mem 0xc0ffee00 flags 0x200]", + tc(test, "[mem 0xc0ffee00 flags 0x200]", "%pr", &test_resource);
test_resource = (struct resource) { @@ -400,7 +349,7 @@ struct_resource(void) .end = 0xba5eba11, .flags = IORESOURCE_MEM, }; - test("[mem 0x00c0ffee-0xba5eba11 flags 0x200]", + tc(test, "[mem 0x00c0ffee-0xba5eba11 flags 0x200]", "%pr", &test_resource);
test_resource = (struct resource) { @@ -408,7 +357,7 @@ struct_resource(void) .end = 0xc0ffee, .flags = IORESOURCE_MEM, }; - test("[mem 0xba5eba11-0x00c0ffee flags 0x200]", + tc(test, "[mem 0xba5eba11-0x00c0ffee flags 0x200]", "%pr", &test_resource);
test_resource = (struct resource) { @@ -417,7 +366,7 @@ struct_resource(void) .flags = IORESOURCE_MEM, };
- test("[mem 0xba5eba11-0xba5eca11 flags 0x200]", + tc(test, "[mem 0xba5eba11-0xba5eca11 flags 0x200]", "%pr", &test_resource);
test_resource = (struct resource) { @@ -428,61 +377,61 @@ struct_resource(void) IORESOURCE_UNSET, };
- test("[io size 0x1000 disabled]", + tc(test, "[io size 0x1000 disabled]", "%pR", &test_resource); }
-static void __init -struct_range(void) +static void +struct_range(struct kunit *test) { struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11, 0xc0ffee00ba5eba11); - test("[range 0xc0ffee00ba5eba11]", "%pra", &test_range); + tc(test, "[range 0xc0ffee00ba5eba11]", "%pra", &test_range);
test_range = DEFINE_RANGE(0xc0ffee, 0xba5eba11); - test("[range 0x0000000000c0ffee-0x00000000ba5eba11]", + tc(test, "[range 0x0000000000c0ffee-0x00000000ba5eba11]", "%pra", &test_range);
test_range = DEFINE_RANGE(0xba5eba11, 0xc0ffee); - test("[range 0x00000000ba5eba11-0x0000000000c0ffee]", + tc(test, "[range 0x00000000ba5eba11-0x0000000000c0ffee]", "%pra", &test_range); }
-static void __init +static void addr(void) { }
-static void __init +static void escaped_str(void) { }
-static void __init -hex_string(void) +static void +hex_string(struct kunit *test) { const char buf[3] = {0xc0, 0xff, 0xee};
- test("c0 ff ee|c0:ff:ee|c0-ff-ee|c0ffee", + tc(test, "c0 ff ee|c0:ff:ee|c0-ff-ee|c0ffee", "%3ph|%3phC|%3phD|%3phN", buf, buf, buf, buf); - test("c0 ff ee|c0:ff:ee|c0-ff-ee|c0ffee", + tc(test, "c0 ff ee|c0:ff:ee|c0-ff-ee|c0ffee", "%*ph|%*phC|%*phD|%*phN", 3, buf, 3, buf, 3, buf, 3, buf); }
-static void __init -mac(void) +static void +mac(struct kunit *test) { const u8 addr[6] = {0x2d, 0x48, 0xd6, 0xfc, 0x7a, 0x05};
- test("2d:48:d6:fc:7a:05", "%pM", addr); - test("05:7a:fc:d6:48:2d", "%pMR", addr); - test("2d-48-d6-fc-7a-05", "%pMF", addr); - test("2d48d6fc7a05", "%pm", addr); - test("057afcd6482d", "%pmR", addr); + tc(test, "2d:48:d6:fc:7a:05", "%pM", addr); + tc(test, "05:7a:fc:d6:48:2d", "%pMR", addr); + tc(test, "2d-48-d6-fc-7a-05", "%pMF", addr); + tc(test, "2d48d6fc7a05", "%pm", addr); + tc(test, "057afcd6482d", "%pmR", addr); }
-static void __init -ip4(void) +static void +ip4(struct kunit *test) { struct sockaddr_in sa;
@@ -490,37 +439,37 @@ ip4(void) sa.sin_port = cpu_to_be16(12345); sa.sin_addr.s_addr = cpu_to_be32(0x7f000001);
- test("127.000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); - test("127.000.000.001|127.0.0.1", "%piS|%pIS", &sa, &sa); + tc(test, "127.000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); + tc(test, "127.000.000.001|127.0.0.1", "%piS|%pIS", &sa, &sa); sa.sin_addr.s_addr = cpu_to_be32(0x01020304); - test("001.002.003.004:12345|1.2.3.4:12345", "%piSp|%pISp", &sa, &sa); + tc(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 -ip(void) +static void +ip(struct kunit *test) { - ip4(); + ip4(test); ip6(); }
-static void __init -uuid(void) +static void +uuid(struct kunit *test) { const char uuid[16] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf};
- test("00010203-0405-0607-0809-0a0b0c0d0e0f", "%pUb", uuid); - test("00010203-0405-0607-0809-0A0B0C0D0E0F", "%pUB", uuid); - test("03020100-0504-0706-0809-0a0b0c0d0e0f", "%pUl", uuid); - test("03020100-0504-0706-0809-0A0B0C0D0E0F", "%pUL", uuid); + tc(test, "00010203-0405-0607-0809-0a0b0c0d0e0f", "%pUb", uuid); + tc(test, "00010203-0405-0607-0809-0A0B0C0D0E0F", "%pUB", uuid); + tc(test, "03020100-0504-0706-0809-0a0b0c0d0e0f", "%pUl", uuid); + tc(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,34 +484,34 @@ static struct dentry test_dentry[4] __initdata = { .d_iname = "romeo" }, };
-static void __init -dentry(void) +static void +dentry(struct kunit *test) { - test("foo", "%pd", &test_dentry[0]); - test("foo", "%pd2", &test_dentry[0]); + tc(test, "foo", "%pd", &test_dentry[0]); + tc(test, "foo", "%pd2", &test_dentry[0]);
- test("(null)", "%pd", NULL); - test("(efault)", "%pd", PTR_INVALID); - test("(null)", "%pD", NULL); - test("(efault)", "%pD", PTR_INVALID); + tc(test, "(null)", "%pd", NULL); + tc(test, "(efault)", "%pd", PTR_INVALID); + tc(test, "(null)", "%pD", NULL); + tc(test, "(efault)", "%pD", PTR_INVALID);
- test("romeo", "%pd", &test_dentry[3]); - test("alfa/romeo", "%pd2", &test_dentry[3]); - test("bravo/alfa/romeo", "%pd3", &test_dentry[3]); - test("/bravo/alfa/romeo", "%pd4", &test_dentry[3]); - test("/bravo/alfa", "%pd4", &test_dentry[2]); + tc(test, "romeo", "%pd", &test_dentry[3]); + tc(test, "alfa/romeo", "%pd2", &test_dentry[3]); + tc(test, "bravo/alfa/romeo", "%pd3", &test_dentry[3]); + tc(test, "/bravo/alfa/romeo", "%pd4", &test_dentry[3]); + tc(test, "/bravo/alfa", "%pd4", &test_dentry[2]);
- test("bravo/alfa |bravo/alfa ", "%-12pd2|%*pd2", &test_dentry[2], -12, &test_dentry[2]); - test(" bravo/alfa| bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]); + tc(test, "bravo/alfa |bravo/alfa ", "%-12pd2|%*pd2", &test_dentry[2], -12, &test_dentry[2]); + tc(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 -time_and_date(void) +static void +time_and_date(struct kunit *test) { /* 1543210543 */ const struct rtc_time tm = { @@ -576,32 +525,32 @@ time_and_date(void) /* 2019-01-04T15:32:23 */ time64_t t = 1546615943;
- test("(%pt?)", "%pt", &tm); - test("2018-11-26T05:35:43", "%ptR", &tm); - test("0118-10-26T05:35:43", "%ptRr", &tm); - test("05:35:43|2018-11-26", "%ptRt|%ptRd", &tm, &tm); - test("05:35:43|0118-10-26", "%ptRtr|%ptRdr", &tm, &tm); - test("05:35:43|2018-11-26", "%ptRttr|%ptRdtr", &tm, &tm); - test("05:35:43 tr|2018-11-26 tr", "%ptRt tr|%ptRd tr", &tm, &tm); + tc(test, "(%pt?)", "%pt", &tm); + tc(test, "2018-11-26T05:35:43", "%ptR", &tm); + tc(test, "0118-10-26T05:35:43", "%ptRr", &tm); + tc(test, "05:35:43|2018-11-26", "%ptRt|%ptRd", &tm, &tm); + tc(test, "05:35:43|0118-10-26", "%ptRtr|%ptRdr", &tm, &tm); + tc(test, "05:35:43|2018-11-26", "%ptRttr|%ptRdtr", &tm, &tm); + tc(test, "05:35:43 tr|2018-11-26 tr", "%ptRt tr|%ptRd tr", &tm, &tm);
- test("2019-01-04T15:32:23", "%ptT", &t); - test("0119-00-04T15:32:23", "%ptTr", &t); - test("15:32:23|2019-01-04", "%ptTt|%ptTd", &t, &t); - test("15:32:23|0119-00-04", "%ptTtr|%ptTdr", &t, &t); + tc(test, "2019-01-04T15:32:23", "%ptT", &t); + tc(test, "0119-00-04T15:32:23", "%ptTr", &t); + tc(test, "15:32:23|2019-01-04", "%ptTt|%ptTd", &t, &t); + tc(test, "15:32:23|0119-00-04", "%ptTtr|%ptTdr", &t, &t);
- test("2019-01-04 15:32:23", "%ptTs", &t); - test("0119-00-04 15:32:23", "%ptTsr", &t); - test("15:32:23|2019-01-04", "%ptTts|%ptTds", &t, &t); - test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t); + tc(test, "2019-01-04 15:32:23", "%ptTs", &t); + tc(test, "0119-00-04 15:32:23", "%ptTsr", &t); + tc(test, "15:32:23|2019-01-04", "%ptTts|%ptTds", &t, &t); + tc(test, "15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t); }
-static void __init +static void struct_clk(void) { }
-static void __init -large_bitmap(void) +static void +large_bitmap(struct kunit *test) { const int nbits = 1 << 16; unsigned long *bits = bitmap_zalloc(nbits, GFP_KERNEL); @@ -610,34 +559,34 @@ large_bitmap(void)
bitmap_set(bits, 1, 20); bitmap_set(bits, 60000, 15); - test("1-20,60000-60014", "%*pbl", nbits, bits); + tc(test, "1-20,60000-60014", "%*pbl", nbits, bits); bitmap_free(bits); }
-static void __init -bitmap(void) +static void +bitmap(struct kunit *test) { DECLARE_BITMAP(bits, 20); const int primes[] = {2,3,5,7,11,13,17,19}; int i;
bitmap_zero(bits, 20); - test("00000|00000", "%20pb|%*pb", bits, 20, bits); - test("|", "%20pbl|%*pbl", bits, 20, bits); + tc(test, "00000|00000", "%20pb|%*pb", bits, 20, bits); + tc(test, "|", "%20pbl|%*pbl", bits, 20, bits);
for (i = 0; i < ARRAY_SIZE(primes); ++i) set_bit(primes[i], bits); - test("a28ac|a28ac", "%20pb|%*pb", bits, 20, bits); - test("2-3,5,7,11,13,17,19|2-3,5,7,11,13,17,19", "%20pbl|%*pbl", bits, 20, bits); + tc(test, "a28ac|a28ac", "%20pb|%*pb", bits, 20, bits); + tc(test, "2-3,5,7,11,13,17,19|2-3,5,7,11,13,17,19", "%20pbl|%*pbl", bits, 20, bits);
bitmap_fill(bits, 20); - test("fffff|fffff", "%20pb|%*pb", bits, 20, bits); - test("0-19|0-19", "%20pbl|%*pbl", bits, 20, bits); + tc(test, "fffff|fffff", "%20pb|%*pb", bits, 20, bits); + tc(test, "0-19|0-19", "%20pbl|%*pbl", bits, 20, bits);
- large_bitmap(); + large_bitmap(test); }
-static void __init +static void netdev_features(void) { } @@ -663,8 +612,8 @@ static const struct page_flags_test pft[] = { "%#x", "kasantag"}, };
-static void __init -page_flags_test(int section, int node, int zone, int last_cpupid, +static void +page_flags_test(struct kunit *test, int section, int node, int zone, int last_cpupid, int kasan_tag, unsigned long flags, const char *name, char *cmp_buf) { @@ -698,11 +647,11 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
snprintf(cmp_buf + size, BUF_SIZE - size, ")");
- test(cmp_buf, "%pGp", &flags); + tc(test, cmp_buf, "%pGp", &flags); }
-static void __init -flags(void) +static void +flags(struct kunit *test) { unsigned long flags; char *cmp_buffer; @@ -713,43 +662,43 @@ flags(void) return;
flags = 0; - page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer); + page_flags_test(test, 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(test, 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(test, 1, 1, 1, 0x1fffff, 1, flags, "uptodate|dirty|lru|active|swapbacked", cmp_buffer);
flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC; - test("read|exec|mayread|maywrite|mayexec", "%pGv", &flags); + tc(test, "read|exec|mayread|maywrite|mayexec", "%pGv", &flags);
gfp = GFP_TRANSHUGE; - test("GFP_TRANSHUGE", "%pGg", &gfp); + tc(test, "GFP_TRANSHUGE", "%pGg", &gfp);
gfp = GFP_ATOMIC|__GFP_DMA; - test("GFP_ATOMIC|GFP_DMA", "%pGg", &gfp); + tc(test, "GFP_ATOMIC|GFP_DMA", "%pGg", &gfp);
gfp = __GFP_HIGH; - test("__GFP_HIGH", "%pGg", &gfp); + tc(test, "__GFP_HIGH", "%pGg", &gfp);
/* Any flags not translated by the table should remain numeric */ gfp = ~__GFP_BITS_MASK; snprintf(cmp_buffer, BUF_SIZE, "%#lx", (unsigned long) gfp); - test(cmp_buffer, "%pGg", &gfp); + tc(test, cmp_buffer, "%pGg", &gfp);
snprintf(cmp_buffer, BUF_SIZE, "__GFP_HIGH|%#lx", (unsigned long) gfp); gfp |= __GFP_HIGH; - test(cmp_buffer, "%pGg", &gfp); + tc(test, cmp_buffer, "%pGg", &gfp);
kfree(cmp_buffer); }
-static void __init fwnode_pointer(void) +static void fwnode_pointer(struct kunit *test) { const struct software_node first = { .name = "first" }; const struct software_node second = { .name = "second", .parent = &first }; @@ -767,16 +716,16 @@ static void __init fwnode_pointer(void) return; }
- test(full_name_second, "%pfw", software_node_fwnode(&second)); - test(full_name_third, "%pfw", software_node_fwnode(&third)); - test(full_name_third, "%pfwf", software_node_fwnode(&third)); - test(second_name, "%pfwP", software_node_fwnode(&second)); - test(third_name, "%pfwP", software_node_fwnode(&third)); + tc(test, full_name_second, "%pfw", software_node_fwnode(&second)); + tc(test, full_name_third, "%pfw", software_node_fwnode(&third)); + tc(test, full_name_third, "%pfwf", software_node_fwnode(&third)); + tc(test, second_name, "%pfwP", software_node_fwnode(&second)); + tc(test, third_name, "%pfwP", software_node_fwnode(&third));
software_node_unregister_node_group(group); }
-static void __init fourcc_pointer(void) +static void fourcc_pointer(struct kunit *test) { struct { u32 code; @@ -790,74 +739,85 @@ static void __init fourcc_pointer(void) unsigned int i;
for (i = 0; i < ARRAY_SIZE(try); i++) - test(try[i].str, "%p4cc", &try[i].code); + tc(test, try[i].str, "%p4cc", &try[i].code); }
-static void __init -errptr(void) +static void +errptr(struct kunit *test) { - test("-1234", "%pe", ERR_PTR(-1234)); + tc(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(test, "%pe", PTR);
#ifdef CONFIG_SYMBOLIC_ERRNAME - test("(-ENOTSOCK)", "(%pe)", ERR_PTR(-ENOTSOCK)); - test("(-EAGAIN)", "(%pe)", ERR_PTR(-EAGAIN)); + tc(test, "(-ENOTSOCK)", "(%pe)", ERR_PTR(-ENOTSOCK)); + tc(test, "(-EAGAIN)", "(%pe)", ERR_PTR(-EAGAIN)); BUILD_BUG_ON(EAGAIN != EWOULDBLOCK); - test("(-EAGAIN)", "(%pe)", ERR_PTR(-EWOULDBLOCK)); - test("[-EIO ]", "[%-8pe]", ERR_PTR(-EIO)); - test("[ -EIO]", "[%8pe]", ERR_PTR(-EIO)); - test("-EPROBE_DEFER", "%pe", ERR_PTR(-EPROBE_DEFER)); + tc(test, "(-EAGAIN)", "(%pe)", ERR_PTR(-EWOULDBLOCK)); + tc(test, "[-EIO ]", "[%-8pe]", ERR_PTR(-EIO)); + tc(test, "[ -EIO]", "[%8pe]", ERR_PTR(-EIO)); + tc(test, "-EPROBE_DEFER", "%pe", ERR_PTR(-EPROBE_DEFER)); #endif }
-static void __init -test_pointer(void) +static void +test_pointer(struct kunit *test) { - plain(); - null_pointer(); - error_pointer(); - invalid_pointer(); + plain(test); + null_pointer(test); + error_pointer(test); + invalid_pointer(test); symbol_ptr(); kernel_ptr(); - struct_resource(); - struct_range(); + struct_resource(test); + struct_range(test); addr(); escaped_str(); - hex_string(); - mac(); - ip(); - uuid(); - dentry(); + hex_string(test); + mac(test); + ip(test); + uuid(test); + dentry(test); struct_va_format(); - time_and_date(); + time_and_date(test); struct_clk(); - bitmap(); + bitmap(test); netdev_features(); - flags(); - errptr(); - fwnode_pointer(); - fourcc_pointer(); + flags(test); + errptr(test); + fwnode_pointer(test); + fourcc_pointer(test); }
-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;
- test_basic(); - test_number(); - test_string(); - test_pointer(); + test_basic(test); + test_number(test); + test_string(test); + test_pointer(test);
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 Tue, 4 Feb 2025 at 20:38, 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.
Signed-off-by: Tamir Duberstein tamird@gmail.com
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 -
Acked-by: Geert Uytterhoeven geert@linux-m68k.org # m68k
Gr{oetje,eeting}s,
Geert
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 | 196 ++++++++++++++++++----------------------------------- 1 file changed, 65 insertions(+), 131 deletions(-)
diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c index e889aca69eba..e88b70ad3830 100644 --- a/lib/printf_kunit.c +++ b/lib/printf_kunit.c @@ -180,29 +180,6 @@ test_string(struct kunit *test) #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 @@ -212,79 +189,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 *test, 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(test, 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) +static void +hash_pointer(struct kunit *test) { - char buf[PLAIN_BUF_SIZE]; - int ret; + if (no_hash_pointers) + kunit_skip(test, "hash pointers disabled");
- ret = plain_hash_to_buffer(PTR, buf, PLAIN_BUF_SIZE); - if (ret) - return ret; + char buf[PLAIN_BUF_SIZE];
- if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0) - return -1; + plain_hash_to_buffer(test, PTR, buf, PLAIN_BUF_SIZE);
- 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(struct kunit *test) -{ - if (no_hash_pointers) { - pr_warn("skipping plain 'p' tests"); - return; - } + /* + * We can't use test() to test %p because we don't know what output to expect + * after an address is hashed. + */
- KUNIT_EXPECT_FALSE(test, plain_hash()); - KUNIT_EXPECT_FALSE(test, plain_format()); + KUNIT_EXPECT_MEMEQ(test, buf, ZEROS, strlen(ZEROS)); + KUNIT_EXPECT_MEMNEQ(test, buf+strlen(ZEROS), PTR_STR, PTR_WIDTH); }
static void test_hashed(struct kunit *test, 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(test, p, buf, PLAIN_BUF_SIZE);
tc(test, buf, fmt, p); } @@ -322,12 +264,12 @@ invalid_pointer(struct kunit *test) }
static void -symbol_ptr(void) +symbol_ptr(struct kunit *test) { }
static void -kernel_ptr(void) +kernel_ptr(struct kunit *test) { /* We can't test this without access to kptr_restrict. */ } @@ -398,12 +340,12 @@ struct_range(struct kunit *test) }
static void -addr(void) +addr(struct kunit *test) { }
static void -escaped_str(void) +escaped_str(struct kunit *test) { }
@@ -446,15 +388,8 @@ ip4(struct kunit *test) }
static void -ip6(void) -{ -} - -static void -ip(struct kunit *test) +ip6(struct kunit *test) { - ip4(test); - ip6(); }
static void @@ -506,7 +441,7 @@ dentry(struct kunit *test) }
static void -struct_va_format(void) +struct_va_format(struct kunit *test) { }
@@ -545,7 +480,7 @@ time_and_date(struct kunit *test) }
static void -struct_clk(void) +struct_clk(struct kunit *test) { }
@@ -587,7 +522,7 @@ bitmap(struct kunit *test) }
static void -netdev_features(void) +netdev_features(struct kunit *test) { }
@@ -712,8 +647,7 @@ static void fwnode_pointer(struct kunit *test)
rval = software_node_register_node_group(group); if (rval) { - pr_warn("cannot register softnodes; rval %d\n", rval); - return; + kunit_skip(test, "cannot register softnodes; rval %d\n", rval); }
tc(test, full_name_second, "%pfw", software_node_fwnode(&second)); @@ -762,57 +696,57 @@ errptr(struct kunit *test) #endif }
-static void -test_pointer(struct kunit *test) -{ - plain(test); - null_pointer(test); - error_pointer(test); - invalid_pointer(test); - symbol_ptr(); - kernel_ptr(); - struct_resource(test); - struct_range(test); - addr(); - escaped_str(); - hex_string(test); - mac(test); - ip(test); - uuid(test); - dentry(test); - struct_va_format(); - time_and_date(test); - struct_clk(); - bitmap(test); - netdev_features(); - flags(test); - errptr(test); - fwnode_pointer(test); - fourcc_pointer(test); -} - -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; +}
- test_basic(test); - test_number(test); - test_string(test); - test_pointer(test); - +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, };
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com wrote:
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]
Sorry, but NAK, not in this form.
Please read the previous threads to understand what is wrong with this mechanical approach. Not only is it wrong, it also actively makes the test suite much less useful.
https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), but I don't know what happened to it.
Rasmus
On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com wrote:
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]
Sorry, but NAK, not in this form.
Please read the previous threads to understand what is wrong with this mechanical approach. Not only is it wrong, it also actively makes the test suite much less useful.
https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), but I don't know what happened to it.
Rasmus
Thanks Rasmus, I wasn't aware of that prior effort. I've gone through and adopted your comments - the result is a first patch that is much smaller (104 insertions(+), 104 deletions(-)) and failure messages that are quite close to what is emitted now. I've taken care to keep all the control flow the same, as you requested. The previous discussion concluded with a promise to send another patch which didn't happen. May I send a v2 with these changes, or are there more fundamental objections? I'll also cc Arpitha and Brendan. The new failure output:
# ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 vsnprintf(buf, 256, "%piS|%pIS", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 vsnprintf(buf, 19, "%piS|%pIS", ...) wrote '127.000.000.001|12', expected '127-000.000.001|12' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:131 kvasprintf(..., "%piS|%pIS", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
Cheers, Tamir
On Thu, 6 Feb 2025 at 23:42, Tamir Duberstein tamird@gmail.com wrote:
On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com wrote:
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]
Sorry, but NAK, not in this form.
Please read the previous threads to understand what is wrong with this mechanical approach. Not only is it wrong, it also actively makes the test suite much less useful.
https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), but I don't know what happened to it.
Rasmus
Thanks Rasmus, I wasn't aware of that prior effort. I've gone through and adopted your comments - the result is a first patch that is much smaller (104 insertions(+), 104 deletions(-)) and failure messages that are quite close to what is emitted now. I've taken care to keep all the control flow the same, as you requested. The previous discussion concluded with a promise to send another patch which didn't happen. May I send a v2 with these changes, or are there more fundamental objections? I'll also cc Arpitha and Brendan. The new failure output:
# ip4: EXPECTATION FAILED at lib/printf_kunit.c:95vsnprintf(buf, 256, "%piS|%pIS", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 vsnprintf(buf, 19, "%piS|%pIS", ...) wrote '127.000.000.001|12', expected '127-000.000.001|12' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:131 kvasprintf(..., "%piS|%pIS", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
This failure message looks good to me. The ones in the current patch are very verbose, and while the memory comparisons could be useful for the overflow/buffer size tests, for simple string comparisons, having the string in a readable format is best.
Rasmus: the KUnit framework has since added a summary line to the output by default, which should also make this less of a regression from the existing format: # printf: pass:28 fail:0 skip:0 total:28
Cheers, -- David
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com wrote:
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]
Sorry, but NAK, not in this form.
Please read the previous threads to understand what is wrong with this mechanical approach. Not only is it wrong, it also actively makes the test suite much less useful.
https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), but I don't know what happened to it.
Rasmus
Thanks Rasmus, I wasn't aware of that prior effort. I've gone through and adopted your comments - the result is a first patch that is much smaller (104 insertions(+), 104 deletions(-)) and failure messages that are quite close to what is emitted now. I've taken care to keep all the control flow the same, as you requested. The previous discussion concluded with a promise to send another patch which didn't happen. May I send a v2 with these changes, or are there more fundamental objections? I'll also cc Arpitha and Brendan. The new failure output:
# ip4: EXPECTATION FAILED at lib/printf_kunit.c:95vsnprintf(buf, 256, "%piS|%pIS", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 vsnprintf(buf, 19, "%piS|%pIS", ...) wrote '127.000.000.001|12', expected '127-000.000.001|12' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:131 kvasprintf(..., "%piS|%pIS", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
That certainly addresses one of my main objections; I really don't want to see "memcmp(..., ...) == 1, expected memcmp(..., ...) == 0". And you said you've kept the control flow/early returns the same, so that should also be ok.
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
The other thing I want to know is if this would make it harder for me to finish up that "deterministic random testing" thing I wrote [*], but never got around to actually get it upstream. It seems like something that a framework like kunit could usefully provide out-of-the-box (which is why I attempted to get it into kselftest), but as long as I'd still be able to add in something like that, and get a "xxx failed, random seed used was 0xabcdef" line printed, and run the test again setting the seed explicitly to that 0xabcdef value, I'm good.
[*] https://lore.kernel.org/lkml/20201025214842.5924-4-linux@rasmusvillemoes.dk/
Rasmus
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com wrote:
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]
Sorry, but NAK, not in this form.
Please read the previous threads to understand what is wrong with this mechanical approach. Not only is it wrong, it also actively makes the test suite much less useful.
https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), but I don't know what happened to it.
Rasmus
Thanks Rasmus, I wasn't aware of that prior effort. I've gone through and adopted your comments - the result is a first patch that is much smaller (104 insertions(+), 104 deletions(-)) and failure messages that are quite close to what is emitted now. I've taken care to keep all the control flow the same, as you requested. The previous discussion concluded with a promise to send another patch which didn't happen. May I send a v2 with these changes, or are there more fundamental objections? I'll also cc Arpitha and Brendan. The new failure output:
# ip4: EXPECTATION FAILED at lib/printf_kunit.c:95vsnprintf(buf, 256, "%piS|%pIS", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 vsnprintf(buf, 19, "%piS|%pIS", ...) wrote '127.000.000.001|12', expected '127-000.000.001|12' # ip4: EXPECTATION FAILED at lib/printf_kunit.c:131 kvasprintf(..., "%piS|%pIS", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
That certainly addresses one of my main objections; I really don't want to see "memcmp(..., ...) == 1, expected memcmp(..., ...) == 0". And you said you've kept the control flow/early returns the same, so that should also be ok.
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
The other thing I want to know is if this would make it harder for me to finish up that "deterministic random testing" thing I wrote [*], but never got around to actually get it upstream. It seems like something that a framework like kunit could usefully provide out-of-the-box (which is why I attempted to get it into kselftest), but as long as I'd still be able to add in something like that, and get a "xxx failed, random seed used was 0xabcdef" line printed, and run the test again setting the seed explicitly to that 0xabcdef value, I'm good.
[*] https://lore.kernel.org/lkml/20201025214842.5924-4-linux@rasmusvillemoes.dk/
I can't speak for the framework, but it wouldn't be any harder to do in printf itself. I did it this way:
+static struct rnd_state rnd_state; +static u64 seed; + static int printf_suite_init(struct kunit_suite *suite) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) return -1; test_buffer = alloced_buffer + PAD_SIZE; + + seed = get_random_u64(); + prandom_seed_state(&rnd_state, seed); return 0; }
static void printf_suite_exit(struct kunit_suite *suite) { kfree(alloced_buffer); + if (kunit_suite_has_succeeded(suite) == KUNIT_FAILURE) { + pr_info("Seed: %llu\n", seed); + } }
and the result (once I made one of the cases fail):
printf_kunit: Seed: 11480747578984087668 # printf: pass:27 fail:1 skip:0 total:28 # Totals: pass:27 fail:1 skip:0 total:28 not ok 1 printf
Thank you both for engaging with me here. I'll send v2 in a few minutes and we can continue the discussion there. Tamir
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
The other thing I want to know is if this would make it harder for me to finish up that "deterministic random testing" thing I wrote [*], but never got around to actually get it upstream. It seems like something that a framework like kunit could usefully provide out-of-the-box (which is why I attempted to get it into kselftest), but as long as I'd still be able to add in something like that, and get a "xxx failed, random seed used was 0xabcdef" line printed, and run the test again setting the seed explicitly to that 0xabcdef value, I'm good.
[*] https://lore.kernel.org/lkml/20201025214842.5924-4-linux@rasmusvillemoes.dk/
I can't speak for the framework, but it wouldn't be any harder to do in printf itself. I did it this way:
+static struct rnd_state rnd_state; +static u64 seed;
static int printf_suite_init(struct kunit_suite *suite) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) return -1; test_buffer = alloced_buffer + PAD_SIZE;
- seed = get_random_u64();
- prandom_seed_state(&rnd_state, seed); return 0;
}
static void printf_suite_exit(struct kunit_suite *suite) { kfree(alloced_buffer);
- if (kunit_suite_has_succeeded(suite) == KUNIT_FAILURE) {
- pr_info("Seed: %llu\n", seed);
- }
}
and the result (once I made one of the cases fail):
printf_kunit: Seed: 11480747578984087668 # printf: pass:27 fail:1 skip:0 total:28 # Totals: pass:27 fail:1 skip:0 total:28 not ok 1 printf
OK, that's good. I think one of the problems previously was that there no longer was such an _init/_exit pair one could hook into to do the seed logic and afterwards do something depending on the success/fail of the whole thing; that was all hidden away by some KUNIT_ wrapping.
Is it still possible to trivially make that seed into a module parameter, and do the "modprobe test_printf seed=0xabcd", or otherwise inject a module parameter when run/loaded via the kunit framework?
Rasmus
On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
It'd be possible to split things up further into tests, at the cost of it being a more extensive refactoring, if having the more granular count tracked by KUnit were desired. It'd also be possible to make these more explicitly data driven via a parameterised test (so each input/output pair is listed in an array, and automatically gets converted to a KUnit subtest).
There are some advantages to having these counts done by the framework, particularly in that any inconsistencies can be picked up by the tooling. Ultimately, though, it's up to you as to what is most useful.
The other thing I want to know is if this would make it harder for me to finish up that "deterministic random testing" thing I wrote [*], but never got around to actually get it upstream. It seems like something that a framework like kunit could usefully provide out-of-the-box (which is why I attempted to get it into kselftest), but as long as I'd still be able to add in something like that, and get a "xxx failed, random seed used was 0xabcdef" line printed, and run the test again setting the seed explicitly to that 0xabcdef value, I'm good.
[*] https://lore.kernel.org/lkml/20201025214842.5924-4-linux@rasmusvillemoes.dk/
I can't speak for the framework, but it wouldn't be any harder to do in printf itself. I did it this way:
+static struct rnd_state rnd_state; +static u64 seed;
static int printf_suite_init(struct kunit_suite *suite) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) return -1; test_buffer = alloced_buffer + PAD_SIZE;
- seed = get_random_u64();
- prandom_seed_state(&rnd_state, seed); return 0;
}
static void printf_suite_exit(struct kunit_suite *suite) { kfree(alloced_buffer);
- if (kunit_suite_has_succeeded(suite) == KUNIT_FAILURE) {
- pr_info("Seed: %llu\n", seed);
- }
}
and the result (once I made one of the cases fail):
printf_kunit: Seed: 11480747578984087668 # printf: pass:27 fail:1 skip:0 total:28 # Totals: pass:27 fail:1 skip:0 total:28 not ok 1 printf
OK, that's good. I think one of the problems previously was that there no longer was such an _init/_exit pair one could hook into to do the seed logic and afterwards do something depending on the success/fail of the whole thing; that was all hidden away by some KUNIT_ wrapping.
Yeah, KUnit has since added the suite_init/suite_exit functions in order to do this sort of thing. Previously we had an _init/_exit pair, but it was run per-test-case, which doesn't work as well here.
Is it still possible to trivially make that seed into a module parameter, and do the "modprobe test_printf seed=0xabcd", or otherwise inject a module parameter when run/loaded via the kunit framework?
It should be just the same as any other module. As mentioned, one day I'd like to standardise this in KUnit so that we can have this also change the test execution order and fit in with tooling, but I'd definitely support doing this via an ad-hoc parameter in the meantime.
Cheers, -- David
On Tue, Feb 11 2025, David Gow davidgow@google.com wrote:
On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
It'd be possible to split things up further into tests, at the cost of it being a more extensive refactoring, if having the more granular count tracked by KUnit were desired.
I think the problem is that kunit is simply not a good framework to do these kinds of tests in, and certainly it's very hard to retrofit kunit after the fact.
It'd also be possible to make
these more explicitly data driven via a parameterised test (so each input/output pair is listed in an array, and automatically gets converted to a KUnit subtest).
So that "array of input/output" very much doesn't work for these specific tests: We really want the format string/varargs to be checked by the compiler, and besides, there's no way to store the necessary varargs and generate a call from those in an array. Moreover, we verify a lot more than just that the correct string is produced; it's also a matter of the right return value regardless of the passed buffer size, etc.
That's also why is nigh impossible to simply change __test() into (another) macro that expands to something that defines an individual struct kunit_case, because the framework is really built around the notion that each case can be represented by a void function call and the name of the test is the stringification of the function name.
So I don't mind the conversion to kunit if that really helps other people, as long as the basic functionality is still present and doesn't impede future extensions - and certainly I don't want to end up in a situation where somebody adds a new %p extension but cannot really add a test for it because kunit makes that hard.
But I hope you all agree that it doesn't make much _sense_ to consider test_number() and test_string() and so on individual "test cases"; the atomic units of test being done in the printf suite is each invocation of the __test() function, with one specific format string/varargs combination.
Rasmus
On Tue, Feb 11, 2025 at 3:40 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, Feb 11 2025, David Gow davidgow@google.com wrote:
On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
It'd be possible to split things up further into tests, at the cost of it being a more extensive refactoring, if having the more granular count tracked by KUnit were desired.
I think the problem is that kunit is simply not a good framework to do these kinds of tests in, and certainly it's very hard to retrofit kunit after the fact.
It'd also be possible to make
these more explicitly data driven via a parameterised test (so each input/output pair is listed in an array, and automatically gets converted to a KUnit subtest).
So that "array of input/output" very much doesn't work for these specific tests: We really want the format string/varargs to be checked by the compiler, and besides, there's no way to store the necessary varargs and generate a call from those in an array. Moreover, we verify a lot more than just that the correct string is produced; it's also a matter of the right return value regardless of the passed buffer size, etc.
That's also why is nigh impossible to simply change __test() into (another) macro that expands to something that defines an individual struct kunit_case, because the framework is really built around the notion that each case can be represented by a void function call and the name of the test is the stringification of the function name.
So I don't mind the conversion to kunit if that really helps other people, as long as the basic functionality is still present and doesn't impede future extensions - and certainly I don't want to end up in a situation where somebody adds a new %p extension but cannot really add a test for it because kunit makes that hard.
But I hope you all agree that it doesn't make much _sense_ to consider test_number() and test_string() and so on individual "test cases"; the atomic units of test being done in the printf suite is each invocation of the __test() function, with one specific format string/varargs combination.
Rasmus
Rasmus, does v3 meet your needs?
https://lore.kernel.org/all/20250210-printf-kunit-convert-v3-1-ee6ac5500f5e@...
Weirdly the cover letter seems to be missing on lore, should I resend?
On Tue, Feb 11, 2025 at 6:34 AM Tamir Duberstein tamird@gmail.com wrote:
https://lore.kernel.org/all/20250210-printf-kunit-convert-v3-1-ee6ac5500f5e@...
Weirdly the cover letter seems to be missing on lore, should I resend?
It's there now. https://lore.kernel.org/all/20250210-printf-kunit-convert-v3-0-ee6ac5500f5e@...
On Tue, 11 Feb 2025 at 16:40, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, Feb 11 2025, David Gow davidgow@google.com wrote:
On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
It'd be possible to split things up further into tests, at the cost of it being a more extensive refactoring, if having the more granular count tracked by KUnit were desired.
I think the problem is that kunit is simply not a good framework to do these kinds of tests in, and certainly it's very hard to retrofit kunit after the fact.
I think I'd have to disagree on the whole (though I'm admittedly biased in KUnit's favour), but I can definitely see that the printf tests do provide some unique challenges, and that either way, a port would require either some code churn or bloat, a need to reinterpret things (such as what unit a 'test' is), or both.
Ultimately, I don't want to force KUnit on anyone if it's going to make things more difficult, so it's ultimately up to you. My personal feeling is that this could work well as a KUnit test, but due to the churn involved, it may not be worth it if no-one wants to take advantage of the tooling.
It'd also be possible to make
these more explicitly data driven via a parameterised test (so each input/output pair is listed in an array, and automatically gets converted to a KUnit subtest).
So that "array of input/output" very much doesn't work for these specific tests: We really want the format string/varargs to be checked by the compiler, and besides, there's no way to store the necessary varargs and generate a call from those in an array. Moreover, we verify a lot more than just that the correct string is produced; it's also a matter of the right return value regardless of the passed buffer size, etc.
Ah, that makes sense. I suspect with enough work and some friendly compiler developers, this could be make to work, but it definitely doesn't seem worth the effort to me.
That's also why is nigh impossible to simply change __test() into (another) macro that expands to something that defines an individual struct kunit_case, because the framework is really built around the notion that each case can be represented by a void function call and the name of the test is the stringification of the function name.
Yeah: it may be possible to do something with KUnit's parameter generating functions (you can have a function which generates a void* test context, as well as a string test name: this could be a struct with a format string and a va_list), but it's definitely getting complicated.
So I don't mind the conversion to kunit if that really helps other people, as long as the basic functionality is still present and doesn't impede future extensions - and certainly I don't want to end up in a situation where somebody adds a new %p extension but cannot really add a test for it because kunit makes that hard.
But I hope you all agree that it doesn't make much _sense_ to consider test_number() and test_string() and so on individual "test cases"; the atomic units of test being done in the printf suite is each invocation of the __test() function, with one specific format string/varargs combination.
I think this is -- to some extent -- a matter of interpretation. I don't think it's wrong to use KUnit test cases to refer to a "thing being tested" (e.g., a specific format specifier) rather than an "individual invocation": lots of KUnit tests already group very related things together. But given the way there are several "checks" within each __test() invocation mirrors this already, I understand why it'd make sense to keep that as the "test case".
I don't have any immediate plans personally to work on the printf/scanf code, so your opinion here definitely matters more than mine. But if this does end up as a KUnit test, I'll definitely keep an eye on it as part of my regular KUnit test runs.
Cheers, -- David
Rasmus
linux-kselftest-mirror@lists.linaro.org
-
David Gow -
Geert Uytterhoeven -
Rasmus Villemoes -
Tamir Duberstein