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 v5: - Update `do_test` `__printf` annotation (Rasmus Villemoes). - Link to v4: https://lore.kernel.org/r/20250214-printf-kunit-convert-v4-0-c254572f1565@gm...
Changes in v4: - Add patch "implicate test line in failure messages". - Rebase on linux-next, move scanf_kunit.c into lib/tests/. - Link to v3: https://lore.kernel.org/r/20250210-printf-kunit-convert-v3-0-ee6ac5500f5e@gm...
Changes in v3: - Remove extraneous trailing newlines from failure messages. - Replace `pr_warn` with `kunit_warn`. - Drop arch changes. - Remove KUnit boilerplate from CONFIG_PRINTF_KUNIT_TEST help text. - Restore `total_tests` counting. - Remove tc_fail macro in last patch. - Link to v2: https://lore.kernel.org/r/20250207-printf-kunit-convert-v2-0-057b23860823@gm...
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 (3): printf: convert self-test to KUnit printf: break kunit into test cases printf: implicate test line in failure messages
Documentation/core-api/printk-formats.rst | 4 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 1 - lib/tests/Makefile | 1 + lib/{test_printf.c => tests/printf_kunit.c} | 437 ++++++++++++---------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 8 files changed, 200 insertions(+), 262 deletions(-) --- base-commit: d4b0fd87ff0d4338b259dc79b2b3c6f7e70e8afa 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 | 4 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 1 - lib/tests/Makefile | 1 + lib/{test_printf.c => tests/printf_kunit.c} | 188 +++++++++++++++------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 8 files changed, 117 insertions(+), 96 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index ecccc0473da9..4bdc394e86af 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 -one or more test cases, if at all feasible. +If you add other %p extensions, please extend <lib/tests/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 f076360ce3c6..b051ccf6b276 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25510,8 +25510,8 @@ R: Sergey Senozhatsky senozhatsky@chromium.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git F: Documentation/core-api/printk-formats.rst -F: lib/test_printf.c F: lib/test_scanf.c +F: lib/tests/printf_kunit.c F: lib/vsprintf.c
VT1211 HARDWARE MONITOR DRIVER diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 7ddbfdacf895..d2b15f633227 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2436,6 +2436,15 @@ 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 runtime. + + If unsure, say N. + config STRING_KUNIT_TEST tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS depends on KUNIT @@ -2449,9 +2458,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 961aef91d493..f31e6a3100ba 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.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_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/tests/Makefile b/lib/tests/Makefile index 8961fbcff7a4..183c6a838a5d 100644 --- a/lib/tests/Makefile +++ b/lib/tests/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o +obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o obj-$(CONFIG_TEST_SORT) += test_sort.o diff --git a/lib/test_printf.c b/lib/tests/printf_kunit.c similarity index 87% rename from lib/test_printf.c rename to lib/tests/printf_kunit.c index 59dbe4f9a4cb..287bbfb61148 100644 --- a/lib/test_printf.c +++ b/lib/tests/printf_kunit.c @@ -3,9 +3,7 @@ * Test cases for printf facility. */
-#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 +23,6 @@
#include <linux/property.h>
-#include "../tools/testing/selftests/kselftest_module.h" - #define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,12 +33,17 @@ block \ __diag_pop();
-KSTM_MODULE_GLOBALS(); +static unsigned int total_tests; + +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) { @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen, 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", 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", + 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", - fmt); - return 1; + tc_fail("vsnprintf(buf, 0, "%s", ...) wrote to buffer", fmt); } - 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", 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", 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", 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'", 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 +109,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'.", + elen, fmt); return; }
@@ -124,19 +122,18 @@ __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'", fmt, p, expect); - failed_tests++; } kfree(p); } @@ -146,7 +143,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 +155,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 +177,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 +215,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]; @@ -230,7 +227,7 @@ plain_format(void) return -1;
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { - pr_warn("crng possibly not yet initialized. plain 'p' buffer contains "%s"", + kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains "%s"", PTR_VAL_NO_CRNG); return 0; } @@ -250,7 +247,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 +256,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; @@ -270,7 +267,7 @@ plain_hash_to_buffer(const void *p, char *buf, size_t len) return -1;
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { - pr_warn("crng possibly not yet initialized. plain 'p' buffer contains "%s"", + kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains "%s"", PTR_VAL_NO_CRNG); return 0; } @@ -278,7 +275,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 +295,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; + kunit_warn(kunittest, "skipping plain 'p' tests"); 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"); 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"); } }
-static void __init +static void test_hashed(const char *fmt, const void *p) { char buf[PLAIN_BUF_SIZE]; @@ -343,7 +337,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 +348,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 +358,7 @@ error_pointer(void)
#define PTR_INVALID ((void *)0x000000ab)
-static void __init +static void invalid_pointer(void) { test_hashed("%p", PTR_INVALID); @@ -372,18 +366,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 +426,7 @@ struct_resource(void) "%pR", &test_resource); }
-static void __init +static void struct_range(void) { struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11, @@ -448,17 +442,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 +463,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 +475,7 @@ mac(void) test("057afcd6482d", "%pmR", addr); }
-static void __init +static void ip4(void) { struct sockaddr_in sa; @@ -496,19 +490,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 +514,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 +529,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 +550,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 +589,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 +608,7 @@ large_bitmap(void) bitmap_free(bits); }
-static void __init +static void bitmap(void) { DECLARE_BITMAP(bits, 20); @@ -637,7 +631,7 @@ bitmap(void) large_bitmap(); }
-static void __init +static void netdev_features(void) { } @@ -663,7 +657,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 +695,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 +743,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 }; @@ -763,7 +757,7 @@ static void __init fwnode_pointer(void)
rval = software_node_register_node_group(group); if (rval) { - pr_warn("cannot register softnodes; rval %d\n", rval); + kunit_warn(kunittest, "cannot register softnodes; rval %d", rval); return; }
@@ -776,7 +770,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 +787,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 +807,7 @@ errptr(void) #endif }
-static void __init +static void test_pointer(void) { plain(); @@ -842,13 +836,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 +853,31 @@ static void __init selftest(void) kfree(alloced_buffer); }
-KSTM_MODULE_LOADERS(test_printf); +static int printf_suite_init(struct kunit_suite *suite) +{ + total_tests = 0; + return 0; +} + +static void printf_suite_exit(struct kunit_suite *suite) +{ + kunit_info(suite, "ran %u tests", total_tests); +} + +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, +}; + +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 306a3d4dca98..f4b4b8822241 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_TEST_BITOPS=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 2025-02-21 15:34:30, Tamir Duberstein 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
Documentation/core-api/printk-formats.rst | 4 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 1 - lib/tests/Makefile | 1 + lib/{test_printf.c => tests/printf_kunit.c} | 188 +++++++++++++++------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 8 files changed, 117 insertions(+), 96 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index ecccc0473da9..4bdc394e86af 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 -one or more test cases, if at all feasible. +If you add other %p extensions, please extend <lib/tests/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 f076360ce3c6..b051ccf6b276 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25510,8 +25510,8 @@ R: Sergey Senozhatsky senozhatsky@chromium.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git F: Documentation/core-api/printk-formats.rst -F: lib/test_printf.c F: lib/test_scanf.c +F: lib/tests/printf_kunit.c F: lib/vsprintf.c VT1211 HARDWARE MONITOR DRIVER diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 7ddbfdacf895..d2b15f633227 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2436,6 +2436,15 @@ 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 runtime.If unsure, say N.config STRING_KUNIT_TEST tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS depends on KUNIT @@ -2449,9 +2458,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 961aef91d493..f31e6a3100ba 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.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_TEST_SCANF) += test_scanf.o obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/tests/Makefile b/lib/tests/Makefile index 8961fbcff7a4..183c6a838a5d 100644 --- a/lib/tests/Makefile +++ b/lib/tests/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o +obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o obj-$(CONFIG_TEST_SORT) += test_sort.o diff --git a/lib/test_printf.c b/lib/tests/printf_kunit.c similarity index 87% rename from lib/test_printf.c rename to lib/tests/printf_kunit.c index 59dbe4f9a4cb..287bbfb61148 100644 --- a/lib/test_printf.c +++ b/lib/tests/printf_kunit.c @@ -3,9 +3,7 @@
- Test cases for printf facility.
*/ -#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 +23,6 @@ #include <linux/property.h> -#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,12 +33,17 @@ block \ __diag_pop(); -KSTM_MODULE_GLOBALS(); +static unsigned int total_tests;
+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) { @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen, 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",
1. It looks a bit strange that the 1st patch replaces pr_warn() with tc_fail() which hides KUNIT_FAIL().
And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
It looks like a non-necessary churn.
It would be better to avoid the temporary "tc_fail" and swith to KUNIT_FAIL() already in this patch.
I did not find any comment about this in the earier versions of the patchset.
Is it just a result of the evolution of the patchset or is there any motivation for this?
2. What was the motivation to remove the trailing '\n', please?
It actually makes a difference from the printk() POV. Messages without the trailing '\n' are _not_ flushed to the console until another message is added. The reason is that they might still be appended by pr_cont(). And printk() emits only complete lines to the console.
In general, messages should include the trailing '\n' unless the code wants to append something later or the trailing '\n' is added by another layer of the code. It does not seem to be this case.
bufsize, fmt, ret, elen);
return 1;
return;
[...]
@@ -842,13 +836,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;
I would use here:
KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);
And move the same change for the other kmalloc() location from the 2nd patch.
test_buffer = alloced_buffer + PAD_SIZE;
- kunittest = test;
- test_basic(); test_number(); test_string();
Otherwise, it looks good to me.
Best Regards, Petr
On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:30, Tamir Duberstein 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
Documentation/core-api/printk-formats.rst | 4 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 1 - lib/tests/Makefile | 1 + lib/{test_printf.c => tests/printf_kunit.c} | 188 +++++++++++++++------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 8 files changed, 117 insertions(+), 96 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index ecccc0473da9..4bdc394e86af 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 -one or more test cases, if at all feasible. +If you add other %p extensions, please extend <lib/tests/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 f076360ce3c6..b051ccf6b276 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25510,8 +25510,8 @@ R: Sergey Senozhatsky senozhatsky@chromium.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git F: Documentation/core-api/printk-formats.rst -F: lib/test_printf.c F: lib/test_scanf.c +F: lib/tests/printf_kunit.c F: lib/vsprintf.c
VT1211 HARDWARE MONITOR DRIVER diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 7ddbfdacf895..d2b15f633227 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2436,6 +2436,15 @@ 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 runtime.If unsure, say N.config STRING_KUNIT_TEST tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS depends on KUNIT @@ -2449,9 +2458,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 961aef91d493..f31e6a3100ba 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.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_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o diff --git a/lib/tests/Makefile b/lib/tests/Makefile index 8961fbcff7a4..183c6a838a5d 100644 --- a/lib/tests/Makefile +++ b/lib/tests/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o +obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o obj-$(CONFIG_TEST_SORT) += test_sort.o diff --git a/lib/test_printf.c b/lib/tests/printf_kunit.c similarity index 87% rename from lib/test_printf.c rename to lib/tests/printf_kunit.c index 59dbe4f9a4cb..287bbfb61148 100644 --- a/lib/test_printf.c +++ b/lib/tests/printf_kunit.c @@ -3,9 +3,7 @@
- Test cases for printf facility.
*/
-#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 +23,6 @@
#include <linux/property.h>
-#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -37,12 +33,17 @@ block \ __diag_pop();
-KSTM_MODULE_GLOBALS(); +static unsigned int total_tests;
+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) { @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen, 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",
It looks a bit strange that the 1st patch replaces pr_warn() with tc_fail() which hides KUNIT_FAIL().
And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
It looks like a non-necessary churn.
It would be better to avoid the temporary "tc_fail" and swith to KUNIT_FAIL() already in this patch.
I did not find any comment about this in the earier versions of the patchset.
Is it just a result of the evolution of the patchset or is there any motivation for this?
The motivation was to keep the width of the macro the same in this first patch for ease of review, particularly in the 7 instances where the invocation wraps to a second line. If you prefer I go straight to KUNIT_FAIL, I can make that change.
What was the motivation to remove the trailing '\n', please?
It actually makes a difference from the printk() POV. Messages without the trailing '\n' are _not_ flushed to the console until another message is added. The reason is that they might still be appended by pr_cont(). And printk() emits only complete lines to the console.
In general, messages should include the trailing '\n' unless the code wants to append something later or the trailing '\n' is added by another layer of the code. It does not seem to be this case.
bufsize, fmt, ret, elen);
return 1;
return; }[...]
I noticed in my testing that the trailing \n didn't change the test output, but I didn't know the details you shared about the trailing \n. I'll restore them, unless we jump straight to the KUNIT macros per the discussion above.
@@ -842,13 +836,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;
I would use here:
KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);And move the same change for the other kmalloc() location from the 2nd patch.
I didn't do that here because I was trying to keep this patch as small as possible, and I wrote that in the commit message.
As for using KUNIT_ASSERT_NOT_NULL here, that would have to change back to an error return in the 2nd patch because this code moves into `suite_init`, which is called with `struct kunit_suite` rather than `struct kunit_test`, and KUnit assertion macros do not work with the former (and for good reason, because failures in suite setup cannot be attributed to a particular test case).
So I'd prefer to leave this as is.
test_buffer = alloced_buffer + PAD_SIZE;
kunittest = test;test_basic(); test_number(); test_string();Otherwise, it looks good to me.
Best Regards, Petr
Thank you for the review!
On Thu, Mar 6, 2025 at 9:25 AM Tamir Duberstein tamird@gmail.com wrote:
On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:30, Tamir Duberstein wrote:
Convert the printf() self-test to a KUnit test.
[...]
What was the motivation to remove the trailing '\n', please?
It actually makes a difference from the printk() POV. Messages without the trailing '\n' are _not_ flushed to the console until another message is added. The reason is that they might still be appended by pr_cont(). And printk() emits only complete lines to the console.
In general, messages should include the trailing '\n' unless the code wants to append something later or the trailing '\n' is added by another layer of the code. It does not seem to be this case.
bufsize, fmt, ret, elen);
return 1;
return; }[...]
I noticed in my testing that the trailing \n didn't change the test output, but I didn't know the details you shared about the trailing \n. I'll restore them, unless we jump straight to the KUNIT macros per the discussion above.
Ah, I forgot that `tc_fail` already delegates to KUNIT_FAIL. This was the reason I removed the trailing newlines -- there is a mix of present and absent trailing newlines in KUNIT_* macros, and it's not clear to me what the correct thing is. For instance, the examples in Documentation/dev-tools/kunit/{start,usage}.rst omit the trailing newlines.
On Thu 2025-03-06 09:41:44, Tamir Duberstein wrote:
On Thu, Mar 6, 2025 at 9:25 AM Tamir Duberstein tamird@gmail.com wrote:
On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:30, Tamir Duberstein wrote:
Convert the printf() self-test to a KUnit test.
[...]
What was the motivation to remove the trailing '\n', please?
It actually makes a difference from the printk() POV. Messages without the trailing '\n' are _not_ flushed to the console until another message is added. The reason is that they might still be appended by pr_cont(). And printk() emits only complete lines to the console.
In general, messages should include the trailing '\n' unless the code wants to append something later or the trailing '\n' is added by another layer of the code. It does not seem to be this case.
bufsize, fmt, ret, elen);
return 1;
return; }[...]
I noticed in my testing that the trailing \n didn't change the test output, but I didn't know the details you shared about the trailing \n. I'll restore them, unless we jump straight to the KUNIT macros per the discussion above.
Ah, I forgot that `tc_fail` already delegates to KUNIT_FAIL. This was the reason I removed the trailing newlines -- there is a mix of present and absent trailing newlines in KUNIT_* macros, and it's not clear to me what the correct thing is. For instance, the examples in Documentation/dev-tools/kunit/{start,usage}.rst omit the trailing newlines.
Honestly, I am not able to find how the KUNIT_FAIL() actually prints the message. I can't find how assert_format() is defined.
Anyway, it seems that for example, kunit_warn() prints the messages as is in kunit_log(). It does not add the trailing '\n' on its own.
Also I do not see any empty lines when I add back the trailing '\n' to KUNIT_FAIL() message. This suggests that even KUNIT_FAIL() prints the messages as is and does not add any extra trailing '\n'.
In my opinion, using the trailing '\n' is the right thing to do from the printk() POV. Please, add it back.
Best Regards, Petr
On Fri, Mar 7, 2025 at 10:55 AM Petr Mladek pmladek@suse.com wrote:
Honestly, I am not able to find how the KUNIT_FAIL() actually prints the message. I can't find how assert_format() is defined.
KUNIT_FAIL -> KUNIT_FAIL_ASSERTION -> _KUNIT_FAILED -> __kunit_do_failed_assertion -> kunit_fail -> kunit_print_string_stream -> kunit_err(test, "%s", buf);
So I agree that the trailing newline is just as necessary here as in any other printk.
On Thu 2025-03-06 09:25:43, Tamir Duberstein wrote:
On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:30, Tamir Duberstein 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.
--- a/lib/test_printf.c +++ b/lib/tests/printf_kunit.c @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen, 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",
It looks a bit strange that the 1st patch replaces pr_warn() with tc_fail() which hides KUNIT_FAIL().
And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
It looks like a non-necessary churn.
It would be better to avoid the temporary "tc_fail" and swith to KUNIT_FAIL() already in this patch.
I did not find any comment about this in the earier versions of the patchset.
Is it just a result of the evolution of the patchset or is there any motivation for this?
The motivation was to keep the width of the macro the same in this first patch for ease of review, particularly in the 7 instances where the invocation wraps to a second line. If you prefer I go straight to KUNIT_FAIL, I can make that change.
I see. It might have been useful when the patch removed the trailing '\n'. But you are going to add it back. So there won't be any hidden change. So I would prefer to go straight to KUNIT_FAIL().
@@ -842,13 +836,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;
I would use here:
KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);And move the same change for the other kmalloc() location from the 2nd patch.
I didn't do that here because I was trying to keep this patch as small as possible, and I wrote that in the commit message.
As for using KUNIT_ASSERT_NOT_NULL here, that would have to change back to an error return in the 2nd patch because this code moves into `suite_init`, which is called with `struct kunit_suite` rather than `struct kunit_test`, and KUnit assertion macros do not work with the former (and for good reason, because failures in suite setup cannot be attributed to a particular test case).
I see. KUNIT_ASSERT_NOT_NULL() can't be used in the .suite_exit() callback.
So I'd prefer to leave this as is.
I agree to leave this as is.
Best Regards, Petr
On Fri, Mar 7, 2025 at 11:01 AM Petr Mladek pmladek@suse.com wrote:
On Thu 2025-03-06 09:25:43, Tamir Duberstein wrote:
On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:30, Tamir Duberstein 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.
--- a/lib/test_printf.c +++ b/lib/tests/printf_kunit.c @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen, 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",
It looks a bit strange that the 1st patch replaces pr_warn() with tc_fail() which hides KUNIT_FAIL().
And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
It looks like a non-necessary churn.
It would be better to avoid the temporary "tc_fail" and swith to KUNIT_FAIL() already in this patch.
I did not find any comment about this in the earier versions of the patchset.
Is it just a result of the evolution of the patchset or is there any motivation for this?
The motivation was to keep the width of the macro the same in this first patch for ease of review, particularly in the 7 instances where the invocation wraps to a second line. If you prefer I go straight to KUNIT_FAIL, I can make that change.
I see. It might have been useful when the patch removed the trailing '\n'. But you are going to add it back. So there won't be any hidden change. So I would prefer to go straight to KUNIT_FAIL().
👍 I've restored all the newlines and added a few previously missing ones.
@@ -842,13 +836,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;
I would use here:
KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);And move the same change for the other kmalloc() location from the 2nd patch.
I didn't do that here because I was trying to keep this patch as small as possible, and I wrote that in the commit message.
As for using KUNIT_ASSERT_NOT_NULL here, that would have to change back to an error return in the 2nd patch because this code moves into `suite_init`, which is called with `struct kunit_suite` rather than `struct kunit_test`, and KUnit assertion macros do not work with the former (and for good reason, because failures in suite setup cannot be attributed to a particular test case).
I see. KUNIT_ASSERT_NOT_NULL() can't be used in the .suite_exit() callback.
So I'd prefer to leave this as is.
I agree to leave this as is.
👍
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/tests/printf_kunit.c | 331 +++++++++++++++++------------------------------ 1 file changed, 121 insertions(+), 210 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c index 287bbfb61148..013df6f6dd49 100644 --- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -38,13 +38,8 @@ static unsigned int total_tests; 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; @@ -58,59 +53,64 @@ do_test(int bufsize, const char *expect, int elen, va_end(aq);
if (ret != elen) { - tc_fail("vsnprintf(buf, %d, "%s", ...) returned %d, expected %d", - bufsize, fmt, ret, elen); + KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) returned %d, expected %d", + bufsize, fmt, ret, elen); return; }
if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) { - tc_fail("vsnprintf(buf, %d, "%s", ...) wrote before buffer", - bufsize, fmt); + KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote before buffer", + bufsize, fmt); return; }
if (!bufsize) { if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) { - tc_fail("vsnprintf(buf, 0, "%s", ...) wrote to buffer", fmt); + KUNIT_FAIL(kunittest, "vsnprintf(buf, 0, "%s", ...) wrote to buffer", + fmt); } return; }
written = min(bufsize-1, elen); if (test_buffer[written]) { - tc_fail("vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer", - bufsize, fmt); + KUNIT_FAIL(kunittest, + "vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer", + bufsize, fmt); return; }
if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) { - tc_fail("vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator", - bufsize, fmt); + KUNIT_FAIL(kunittest, + "vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator", + bufsize, fmt); return; }
if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) { - tc_fail("vsnprintf(buf, %d, "%s", ...) wrote beyond buffer", bufsize, fmt); + KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote beyond buffer", + bufsize, fmt); return; }
if (memcmp(test_buffer, expect, written)) { - tc_fail("vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'", - bufsize, fmt, test_buffer, written, expect); + KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'", + bufsize, fmt, test_buffer, written, expect); return; } }
-static void __printf(3, 4) -__test(const char *expect, int elen, const char *fmt, ...) +static void __printf(4, 0) +__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, ...) { va_list ap; int rand; char *p;
if (elen >= BUF_SIZE) { - tc_fail("error in test suite: expected output length %d too long. Format was '%s'.", - elen, fmt); + KUNIT_FAIL(kunittest, + "error in test suite: expected length (%d) >= BUF_SIZE (%d). fmt="%s"", + elen, BUF_SIZE, fmt); return; }
@@ -122,18 +122,19 @@ __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) { total_tests++; if (memcmp(p, expect, elen+1)) { - tc_fail("kvasprintf(..., "%s", ...) returned '%s', expected '%s'", - fmt, p, expect); + KUNIT_FAIL(kunittest, + "kvasprintf(..., "%s", ...) returned '%s', expected '%s'", + fmt, p, expect); } kfree(p); } @@ -141,10 +142,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'; @@ -152,11 +153,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); @@ -178,7 +179,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"); @@ -215,29 +216,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) { - kunit_warn(kunittest, "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 @@ -247,89 +225,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) { kunit_warn(kunittest, "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) { - kunit_warn(kunittest, "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"); - return; - } + plain_hash_to_buffer(kunittest, PTR, buf, PLAIN_BUF_SIZE);
- err = plain_format(); - if (err) { - tc_fail("hashing plain 'p' has unexpected format"); - } + /* + * 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); } @@ -338,7 +271,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); @@ -349,7 +282,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)); @@ -359,26 +292,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, @@ -427,7 +360,7 @@ struct_resource(void) }
static void -struct_range(void) +struct_range(struct kunit *kunittest) { struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11, 0xc0ffee00ba5eba11); @@ -443,17 +376,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};
@@ -464,7 +397,7 @@ hex_string(void) }
static void -mac(void) +mac(struct kunit *kunittest) { const u8 addr[6] = {0x2d, 0x48, 0xd6, 0xfc, 0x7a, 0x05};
@@ -476,7 +409,7 @@ mac(void) }
static void -ip4(void) +ip4(struct kunit *kunittest) { struct sockaddr_in sa;
@@ -491,19 +424,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}; @@ -530,7 +456,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]); @@ -551,12 +477,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 = { @@ -590,12 +516,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); @@ -609,7 +535,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}; @@ -628,11 +554,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) { }
@@ -658,8 +584,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}; @@ -696,25 +622,24 @@ 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; gfp_t gfp;
- cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL); - if (!cmp_buffer) - return; + cmp_buffer = kunit_kmalloc(kunittest, BUF_SIZE, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(kunittest, cmp_buffer);
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);
@@ -739,11 +664,9 @@ flags(void) (unsigned long) gfp); gfp |= __GFP_HIGH; test(cmp_buffer, "%pGg", &gfp); - - 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 }; @@ -757,8 +680,7 @@ static void fwnode_pointer(void)
rval = software_node_register_node_group(group); if (rval) { - kunit_warn(kunittest, "cannot register softnodes; rval %d", rval); - return; + kunit_skip(kunittest, "cannot register softnodes; rval %d", rval); }
test(full_name_second, "%pfw", software_node_fwnode(&second)); @@ -770,7 +692,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; @@ -788,13 +710,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)); @@ -807,65 +729,54 @@ 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 int printf_suite_init(struct kunit_suite *suite) { + total_tests = 0; + alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) - return; + return -ENOMEM; test_buffer = alloced_buffer + PAD_SIZE;
- kunittest = test; - - test_basic(); - test_number(); - test_string(); - test_pointer(); - - kfree(alloced_buffer); -} - -static int printf_suite_init(struct kunit_suite *suite) -{ - total_tests = 0; return 0; }
static void printf_suite_exit(struct kunit_suite *suite) { + kfree(alloced_buffer); + kunit_info(suite, "ran %u tests", total_tests); }
static struct kunit_case printf_test_cases[] = { - KUNIT_CASE(printf_test), + 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), {} };
On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
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/tests/printf_kunit.c | 331 +++++++++++++++++------------------------------ 1 file changed, 121 insertions(+), 210 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c index 287bbfb61148..013df6f6dd49 100644 --- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -38,13 +38,8 @@ static unsigned int total_tests; 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; @@ -58,59 +53,64 @@ do_test(int bufsize, const char *expect, int elen,
[...]
if (memcmp(test_buffer, expect, written)) {
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'",bufsize, fmt, test_buffer, written, expect);
KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'",bufsize, fmt, test_buffer, written, expect);} -static void __printf(3, 4) -__test(const char *expect, int elen, const char *fmt, ...) +static void __printf(4, 0)
This should be:
static void __printf(4, 5)
The 2nd parameter is zero when the variable list of parameters is passed using va_list.
+__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, ...) { va_list ap; int rand; char *p;
@@ -247,89 +225,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) { kunit_warn(kunittest, "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) {
kunit_warn(kunittest, "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");return;- }
- plain_hash_to_buffer(kunittest, PTR, buf, PLAIN_BUF_SIZE);
- err = plain_format();
- if (err) {
tc_fail("hashing plain 'p' has unexpected format");- }
- /*
* We can't use test() to test %p because we don't know what output to expect* after an address is hashed.*/
The code does not longer print a reasonable error message on failure. I would extend the comment to make it easier to understand the meaning. Also I would use the imperative style. Something like:
/* * The hash of %p is unpredictable, therefore test() cannot be used. * Instead, verify that the first 32 bits are zeros on a 64-bit system, * and confirm the non-hashed value is not printed. */
- KUNIT_EXPECT_MEMEQ(kunittest, buf, ZEROS, strlen(ZEROS));
- KUNIT_EXPECT_MEMNEQ(kunittest, buf+strlen(ZEROS), PTR_STR, PTR_WIDTH);
This looks wrong. It should be either:
KUNIT_EXPECT_MEMNEQ(kunittest, buf, PTR_STR, PTR_WIDTH);
or
KUNIT_EXPECT_MEMNEQ(kunittest, buf + strlen(ZEROS), PTR_STR + strlen(ZEROS), PTR_WIDTH - strlen(ZEROS));
I would use the 1st variant. It is easier and it works the same way as the original check.
Anyway, it is a great clean up of the pointer tests. I have wanted to do it since a long time but I never found time.
} 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); } @@ -739,11 +664,9 @@ flags(void) (unsigned long) gfp); gfp |= __GFP_HIGH; test(cmp_buffer, "%pGg", &gfp);
- kfree(cmp_buffer);
I belive that the kfree() should stay. Otherwise, the test leaks memory in every run.
} -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 };
Otherwise, it looks good to me.
Best Regards, Petr
On Thu, Mar 6, 2025 at 11:44 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
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/tests/printf_kunit.c | 331 +++++++++++++++++------------------------------ 1 file changed, 121 insertions(+), 210 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c index 287bbfb61148..013df6f6dd49 100644 --- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -38,13 +38,8 @@ static unsigned int total_tests; 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; @@ -58,59 +53,64 @@ do_test(int bufsize, const char *expect, int elen,
[...]
if (memcmp(test_buffer, expect, written)) {
tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'",bufsize, fmt, test_buffer, written, expect);
KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'",bufsize, fmt, test_buffer, written, expect); return; }}
-static void __printf(3, 4) -__test(const char *expect, int elen, const char *fmt, ...) +static void __printf(4, 0)
This should be:
static void __printf(4, 5)
The 2nd parameter is zero when the variable list of parameters is passed using va_list.
Yeah, thanks for the catch. I fixed this locally after you observed the same on the scanf-kunit series.
+__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, ...) { va_list ap; int rand; char *p;
@@ -247,89 +225,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) { kunit_warn(kunittest, "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) {kunit_warn(kunittest, "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");return;}
plain_hash_to_buffer(kunittest, PTR, buf, PLAIN_BUF_SIZE);
err = plain_format();if (err) {tc_fail("hashing plain 'p' has unexpected format");}
/** We can't use test() to test %p because we don't know what output to expect* after an address is hashed.*/The code does not longer print a reasonable error message on failure. I would extend the comment to make it easier to understand the meaning. Also I would use the imperative style. Something like:
/* * The hash of %p is unpredictable, therefore test() cannot be used. * Instead, verify that the first 32 bits are zeros on a 64-bit system, * and confirm the non-hashed value is not printed. */
I'll make this change. Note that this comment isn't changing here, it only appears to be because its indentation changed.
KUNIT_EXPECT_MEMEQ(kunittest, buf, ZEROS, strlen(ZEROS));KUNIT_EXPECT_MEMNEQ(kunittest, buf+strlen(ZEROS), PTR_STR, PTR_WIDTH);This looks wrong. It should be either:
KUNIT_EXPECT_MEMNEQ(kunittest, buf, PTR_STR, PTR_WIDTH);or
KUNIT_EXPECT_MEMNEQ(kunittest, buf + strlen(ZEROS), PTR_STR + strlen(ZEROS), PTR_WIDTH - strlen(ZEROS));I would use the 1st variant. It is easier and it works the same way as the original check.
Ah, I see. Done as you ask.
Anyway, it is a great clean up of the pointer tests. I have wanted to do it since a long time but I never found time.
Thanks!
}
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);} @@ -739,11 +664,9 @@ flags(void) (unsigned long) gfp); gfp |= __GFP_HIGH; test(cmp_buffer, "%pGg", &gfp);
kfree(cmp_buffer);I belive that the kfree() should stay. Otherwise, the test leaks memory in every run.
This memory is now allocated using `kunit_kmalloc`:
- kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
[...]
- See kmalloc() and kunit_kmalloc_array() for more information.
`kunit_kmalloc_array`:
- Just like `kmalloc_array(...)`, except the allocation is managed by the test case
- and is automatically cleaned up after the test case concludes. See kunit_add_action()
- for more information.
So this kfree is not necessary.
}
-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 };
Otherwise, it looks good to me.
Best Regards, Petr
On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
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.
--- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -178,7 +179,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"); @@ -215,29 +216,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) {
kunit_warn(kunittest, "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 @@ -247,89 +225,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) { kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains "%s"", PTR_VAL_NO_CRNG);
return 0;
I have simulated the not-yet-initialized crng and got:
[ 80.109760] printf_kunit: module verification failed: signature and/or required key missing - tainting kernel [ 80.114218] KTAP version 1 [ 80.114743] 1..1 [ 80.116124] KTAP version 1 [ 80.116752] # Subtest: printf [ 80.117239] # module: printf_kunit [ 80.117256] 1..28 [ 80.120924] ok 1 test_basic [ 80.121495] ok 2 test_number [ 80.122741] ok 3 test_string [ 80.123498] # hash_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.124044] # hash_pointer: EXPECTATION FAILED at lib/tests/printf_kunit.c:256 Expected buf == "00000000", but buf == <28><5f><5f><5f><5f><70><74><72> "00000000" == <30><30><30><30><30><30><30><30> [ 80.125888] not ok 4 hash_pointer [ 80.129831] ok 5 null_pointer [ 80.130253] ok 6 error_pointer [ 80.131221] # invalid_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.132168] ok 7 invalid_pointer [ 80.135149] ok 8 symbol_ptr [ 80.136016] ok 9 kernel_ptr [ 80.136868] ok 10 struct_resource [ 80.137768] ok 11 struct_range [ 80.138613] ok 12 addr [ 80.139370] ok 13 escaped_str [ 80.140054] ok 14 hex_string [ 80.140601] ok 15 mac [ 80.141162] ok 16 ip4 [ 80.141670] ok 17 ip6 [ 80.142221] ok 18 uuid [ 80.143090] ok 19 dentry [ 80.143963] ok 20 struct_va_format [ 80.144523] ok 21 time_and_date [ 80.145043] ok 22 struct_clk [ 80.145589] ok 23 bitmap [ 80.146087] ok 24 netdev_features [ 80.146572] ok 25 flags [ 80.146980] # errptr: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.147412] ok 26 errptr [ 80.148548] ok 27 fwnode_pointer [ 80.149086] ok 28 fourcc_pointer [ 80.149090] # printf: ran 448 tests [ 80.149099] # printf: pass:27 fail:1 skip:0 total:28 [ 80.149102] # Totals: pass:27 fail:1 skip:0 total:28 [ 80.149106] not ok 1 printf
=> One test failed even though vspritf() worked as expected.
The "EXPECTATION FAILED" message was a bit tricky because it printed "<28><5f><5f><5f><5f><70><74><72>" instead of "(____ptrval____)".
Two tests succeeded even after a warning message which would make people to investigate it.
I suggest to rather skip the test in this case. Something like:
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { kunit_skip(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains "%s"\n", PTR_VAL_NO_CRNG); }
It produces:
[ 140.555055] KTAP version 1 [ 140.555413] 1..1 [ 140.555796] KTAP version 1 [ 140.556115] # Subtest: printf [ 140.556450] # module: printf_kunit [ 140.556459] 1..28 [ 140.557757] ok 1 test_basic [ 140.558072] ok 2 test_number [ 140.558693] ok 3 test_string [ 140.559278] ok 4 hash_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.560949] ok 5 null_pointer [ 140.561257] ok 6 error_pointer [ 140.561880] ok 7 invalid_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.564159] ok 8 symbol_ptr [ 140.565248] ok 9 kernel_ptr [ 140.566346] ok 10 struct_resource [ 140.567642] ok 11 struct_range [ 140.569141] ok 12 addr [ 140.570395] ok 13 escaped_str [ 140.571407] ok 14 hex_string [ 140.572337] ok 15 mac [ 140.573572] ok 16 ip4 [ 140.574712] ok 17 ip6 [ 140.575743] ok 18 uuid [ 140.577164] ok 19 dentry [ 140.578248] ok 20 struct_va_format [ 140.579400] ok 21 time_and_date [ 140.580507] ok 22 struct_clk [ 140.581706] ok 23 bitmap [ 140.582739] ok 24 netdev_features [ 140.583808] ok 25 flags [ 140.585274] ok 26 errptr # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.588403] ok 27 fwnode_pointer [ 140.592141] ok 28 fourcc_pointer [ 140.592758] # printf: ran 408 tests [ 140.593219] # printf: pass:25 fail:0 skip:3 total:28 [ 140.593706] # Totals: pass:25 fail:0 skip:3 total:28 [ 140.594280] ok 1 printf
Best Regards, Petr
On Fri, Mar 7, 2025 at 11:18 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
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.
--- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -178,7 +179,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"); @@ -215,29 +216,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) {kunit_warn(kunittest, "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 @@ -247,89 +225,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) { kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains \"%s\"", PTR_VAL_NO_CRNG);
return 0;I have simulated the not-yet-initialized crng and got:
[ 80.109760] printf_kunit: module verification failed: signature and/or required key missing - tainting kernel [ 80.114218] KTAP version 1 [ 80.114743] 1..1 [ 80.116124] KTAP version 1 [ 80.116752] # Subtest: printf [ 80.117239] # module: printf_kunit [ 80.117256] 1..28 [ 80.120924] ok 1 test_basic [ 80.121495] ok 2 test_number [ 80.122741] ok 3 test_string [ 80.123498] # hash_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.124044] # hash_pointer: EXPECTATION FAILED at lib/tests/printf_kunit.c:256 Expected buf == "00000000", but buf == <28><5f><5f><5f><5f><70><74><72> "00000000" == <30><30><30><30><30><30><30><30> [ 80.125888] not ok 4 hash_pointer [ 80.129831] ok 5 null_pointer [ 80.130253] ok 6 error_pointer [ 80.131221] # invalid_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.132168] ok 7 invalid_pointer [ 80.135149] ok 8 symbol_ptr [ 80.136016] ok 9 kernel_ptr [ 80.136868] ok 10 struct_resource [ 80.137768] ok 11 struct_range [ 80.138613] ok 12 addr [ 80.139370] ok 13 escaped_str [ 80.140054] ok 14 hex_string [ 80.140601] ok 15 mac [ 80.141162] ok 16 ip4 [ 80.141670] ok 17 ip6 [ 80.142221] ok 18 uuid [ 80.143090] ok 19 dentry [ 80.143963] ok 20 struct_va_format [ 80.144523] ok 21 time_and_date [ 80.145043] ok 22 struct_clk [ 80.145589] ok 23 bitmap [ 80.146087] ok 24 netdev_features [ 80.146572] ok 25 flags [ 80.146980] # errptr: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 80.147412] ok 26 errptr [ 80.148548] ok 27 fwnode_pointer [ 80.149086] ok 28 fourcc_pointer [ 80.149090] # printf: ran 448 tests [ 80.149099] # printf: pass:27 fail:1 skip:0 total:28 [ 80.149102] # Totals: pass:27 fail:1 skip:0 total:28 [ 80.149106] not ok 1 printf
=> One test failed even though vspritf() worked as expected.
The "EXPECTATION FAILED" message was a bit tricky because it printed "<28><5f><5f><5f><5f><70><74><72>" instead of "(____ptrval____)".
Two tests succeeded even after a warning message which would make people to investigate it.
I suggest to rather skip the test in this case. Something like:
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) { kunit_skip(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains \"%s\"\n", PTR_VAL_NO_CRNG); }It produces:
[ 140.555055] KTAP version 1 [ 140.555413] 1..1 [ 140.555796] KTAP version 1 [ 140.556115] # Subtest: printf [ 140.556450] # module: printf_kunit [ 140.556459] 1..28 [ 140.557757] ok 1 test_basic [ 140.558072] ok 2 test_number [ 140.558693] ok 3 test_string [ 140.559278] ok 4 hash_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.560949] ok 5 null_pointer [ 140.561257] ok 6 error_pointer [ 140.561880] ok 7 invalid_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.564159] ok 8 symbol_ptr [ 140.565248] ok 9 kernel_ptr [ 140.566346] ok 10 struct_resource [ 140.567642] ok 11 struct_range [ 140.569141] ok 12 addr [ 140.570395] ok 13 escaped_str [ 140.571407] ok 14 hex_string [ 140.572337] ok 15 mac [ 140.573572] ok 16 ip4 [ 140.574712] ok 17 ip6 [ 140.575743] ok 18 uuid [ 140.577164] ok 19 dentry [ 140.578248] ok 20 struct_va_format [ 140.579400] ok 21 time_and_date [ 140.580507] ok 22 struct_clk [ 140.581706] ok 23 bitmap [ 140.582739] ok 24 netdev_features [ 140.583808] ok 25 flags [ 140.585274] ok 26 errptr # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)" [ 140.588403] ok 27 fwnode_pointer [ 140.592141] ok 28 fourcc_pointer [ 140.592758] # printf: ran 408 tests [ 140.593219] # printf: pass:25 fail:0 skip:3 total:28 [ 140.593706] # Totals: pass:25 fail:0 skip:3 total:28 [ 140.594280] ok 1 printf
Best Regards, Petr
Thanks for testing! I didn't know how to do that. Changed to kunit_skip as you suggested.
This improves the failure output by pointing to the failing line at the top level of the test, e.g.: # test_number: EXPECTATION FAILED at lib/printf_kunit.c:103 lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote '0x1234abcd ', expected '0x1234abce ' # test_number: EXPECTATION FAILED at lib/printf_kunit.c:142 lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned '0x1234abcd ', expected '0x1234abce '
Signed-off-by: Tamir Duberstein tamird@gmail.com --- lib/tests/printf_kunit.c | 62 ++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c index 013df6f6dd49..ed6e708ddb61 100644 --- a/lib/tests/printf_kunit.c +++ b/lib/tests/printf_kunit.c @@ -38,9 +38,9 @@ static unsigned int total_tests; static char *test_buffer; static char *alloced_buffer;
-static void __printf(5, 0) -do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen, - const char *fmt, va_list ap) +static void __printf(7, 0) +do_test(struct kunit *kunittest, const char *file, const int line, int bufsize, const char *expect, + int elen, const char *fmt, va_list ap) { va_list aq; int ret, written; @@ -53,21 +53,24 @@ do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen, va_end(aq);
if (ret != elen) { - KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) returned %d, expected %d", - bufsize, fmt, ret, elen); + KUNIT_FAIL(kunittest, + "%s:%d: vsnprintf(buf, %d, "%s", ...) returned %d, expected %d", + file, line, bufsize, fmt, ret, elen); return; }
if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) { - KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote before buffer", - bufsize, fmt); + KUNIT_FAIL(kunittest, + "%s:%d: vsnprintf(buf, %d, "%s", ...) wrote before buffer", + file, line, bufsize, fmt); return; }
if (!bufsize) { if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) { - KUNIT_FAIL(kunittest, "vsnprintf(buf, 0, "%s", ...) wrote to buffer", - fmt); + KUNIT_FAIL(kunittest, + "%s:%d: vsnprintf(buf, 0, "%s", ...) wrote to buffer", + file, line, fmt); } return; } @@ -75,33 +78,36 @@ do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen, written = min(bufsize-1, elen); if (test_buffer[written]) { KUNIT_FAIL(kunittest, - "vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer", - bufsize, fmt); + "%s:%d: vsnprintf(buf, %d, "%s", ...) did not nul-terminate buffer", + file, line, bufsize, fmt); return; }
if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) { KUNIT_FAIL(kunittest, - "vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator", - bufsize, fmt); + "%s:%d: vsnprintf(buf, %d, "%s", ...) wrote beyond the nul-terminator", + file, line, bufsize, fmt); return; }
if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) { - KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote beyond buffer", - bufsize, fmt); + KUNIT_FAIL(kunittest, + "%s:%d: vsnprintf(buf, %d, "%s", ...) wrote beyond buffer", + file, line, bufsize, fmt); return; }
if (memcmp(test_buffer, expect, written)) { - KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'", - bufsize, fmt, test_buffer, written, expect); + KUNIT_FAIL(kunittest, + "%s:%d: vsnprintf(buf, %d, "%s", ...) wrote '%s', expected '%.*s'", + file, line, bufsize, fmt, test_buffer, written, expect); return; } }
-static void __printf(4, 0) -__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, ...) +static void __printf(6, 0) +__test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen, + const char *fmt, ...) { va_list ap; int rand; @@ -109,8 +115,8 @@ __test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, .
if (elen >= BUF_SIZE) { KUNIT_FAIL(kunittest, - "error in test suite: expected length (%d) >= BUF_SIZE (%d). fmt="%s"", - elen, BUF_SIZE, fmt); + "%s:%d: error in test suite: expected length (%d) >= BUF_SIZE (%d). fmt="%s"", + file, line, elen, BUF_SIZE, fmt); return; }
@@ -122,19 +128,19 @@ __test(struct kunit *kunittest, 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(kunittest, BUF_SIZE, expect, elen, fmt, ap); + do_test(kunittest, file, line, 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(kunittest, rand, expect, elen, fmt, ap); - do_test(kunittest, 0, expect, elen, fmt, ap); + do_test(kunittest, file, line, rand, expect, elen, fmt, ap); + do_test(kunittest, file, line, 0, expect, elen, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap); if (p) { total_tests++; if (memcmp(p, expect, elen+1)) { KUNIT_FAIL(kunittest, - "kvasprintf(..., "%s", ...) returned '%s', expected '%s'", - fmt, p, expect); + "%s:%d: kvasprintf(..., "%s", ...) returned '%s', expected '%s'", + file, line, fmt, p, expect); } kfree(p); } @@ -142,7 +148,7 @@ __test(struct kunit *kunittest, const char *expect, int elen, const char *fmt, . }
#define test(expect, fmt, ...) \ - __test(kunittest, expect, strlen(expect), fmt, ##__VA_ARGS__) + __test(kunittest, __FILE__, __LINE__, expect, strlen(expect), fmt, ##__VA_ARGS__)
static void test_basic(struct kunit *kunittest) @@ -153,7 +159,7 @@ test_basic(struct kunit *kunittest) test("", &nul); test("100%", "100%%"); test("xxx%yyy", "xxx%cyyy", '%'); - __test(kunittest, "xxx\0yyy", 7, "xxx%cyyy", '\0'); + __test(kunittest, __FILE__, __LINE__, "xxx\0yyy", 7, "xxx%cyyy", '\0'); }
static void
On Fri 2025-02-21 15:34:32, Tamir Duberstein wrote:
This improves the failure output by pointing to the failing line at the top level of the test, e.g.: # test_number: EXPECTATION FAILED at lib/printf_kunit.c:103 lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote '0x1234abcd ', expected '0x1234abce ' # test_number: EXPECTATION FAILED at lib/printf_kunit.c:142 lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned '0x1234abcd ', expected '0x1234abce '
Signed-off-by: Tamir Duberstein tamird@gmail.com
Just for record. I like this improvement. But I am going to wait for the next version of the patchset which is going to add back the trailing '\n'.
Best Regards, Petr
On Fri, Mar 7, 2025 at 11:23 AM Petr Mladek pmladek@suse.com wrote:
On Fri 2025-02-21 15:34:32, Tamir Duberstein wrote:
This improves the failure output by pointing to the failing line at the top level of the test, e.g.: # test_number: EXPECTATION FAILED at lib/printf_kunit.c:103 lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote '0x1234abcd ', expected '0x1234abce ' # test_number: EXPECTATION FAILED at lib/printf_kunit.c:142 lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned '0x1234abcd ', expected '0x1234abce '
Signed-off-by: Tamir Duberstein tamird@gmail.com
Just for record. I like this improvement. But I am going to wait for the next version of the patchset which is going to add back the trailing '\n'.
👍
linux-kselftest-mirror@lists.linaro.org
-
Petr Mladek -
Tamir Duberstein