[PATCH v3 0/2] printf: convert self-test to KUnit

List overview All Threads
Download

newer

older

[PATCH] Adds colors to kselftest...

[PATCH 1/2] misc:...

Tamir Duberstein

10 Feb 2025 10 Feb '25

6:23 p.m.

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 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 (2): printf: convert self-test to KUnit printf: break kunit into test cases

Documentation/core-api/printk-formats.rst | 2 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 429 +++++++++++++----------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 7 files changed, 192 insertions(+), 260 deletions(-) --- base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 change-id: 20250131-printf-kunit-convert-fd4012aa2ec6

Best regards,

-- Tamir Duberstein tamird@gmail.com

Show replies by date

Tamir Duberstein

10 Feb 10 Feb

6:23 p.m.

New subject: [PATCH v3 1/2] printf: convert self-test to KUnit

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 +- lib/Kconfig.debug | 12 +- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 188 +++++++++++++++++------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 7 files changed, 116 insertions(+), 95 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 25c86f47353d..2d58914c7dd9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25411,7 +25411,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/lib/Kconfig.debug b/lib/Kconfig.debug index 1af972a92d06..9f1e0d3513fb 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2427,6 +2427,15 @@ config ASYNC_RAID6_TEST config TEST_HEXDUMP tristate "Test functions located in the hexdump module at runtime"

+config 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 @@ -2440,9 +2449,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 87% rename from lib/test_printf.c rename to lib/printf_kunit.c index 59dbe4f9a4cb..287bbfb61148 100644 --- a/lib/test_printf.c +++ b/lib/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 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

-- 2.48.1

Tamir Duberstein

6:23 p.m.

New subject: [PATCH v3 2/2] printf: break kunit into test cases

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 | 331 ++++++++++++++++++++--------------------------------- 1 file changed, 121 insertions(+), 210 deletions(-)

diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c index 287bbfb61148..00c35eb0abdc 100644 --- a/lib/printf_kunit.c +++ b/lib/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, 5) +__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) -{ -} - -static void -ip(void) +ip6(struct kunit *kunittest) { - 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);

@@ -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), {} };

-- 2.48.1

Petr Mladek

14 Feb 14 Feb

3:35 p.m.

On Mon 2025-02-10 13:23:21, Tamir Duberstein 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]

Signed-off-by: Tamir Duberstein tamird@gmail.com

I have just quickly tested this before leaving for a week. And I am fine with the result.

I tried to simmulate an error.

diff --git a/lib/test_printf.c b/lib/test_printf.c index 59dbe4f9a4cb..d2a1af31a540 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -590,7 +590,7 @@ time_and_date(void) 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("0119-00-04 15:32:24", "%ptTsr", &t); test("15:32:23|2019-01-04", "%ptTts|%ptTds", &t, &t); test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t); }

The original result was:

[ 787.626709] test_printf: loaded. [ 787.627398] test_printf: vsnprintf(buf, 256, "%ptTsr", ...) wrote '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 787.628496] test_printf: kvasprintf(..., "%ptTsr", ...) returned '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 787.629939] test_printf: failed 2 out of 448 tests

The new output is:

[ 585.652278] KTAP version 1 [ 585.652675] 1..1 [ 585.653085] KTAP version 1 [ 585.653382] # Subtest: printf [ 585.653702] # module: printf_kunit [ 585.653716] 1..28 [ 585.655223] ok 1 test_basic [ 585.655908] ok 2 test_number [ 585.656824] ok 3 test_string [ 585.657522] ok 4 hash_pointer [ 585.658547] ok 5 null_pointer [ 585.659572] ok 6 error_pointer [ 585.661057] ok 7 invalid_pointer [ 585.662290] ok 8 symbol_ptr [ 585.663390] ok 9 kernel_ptr [ 585.665162] ok 10 struct_resource [ 585.666231] ok 11 struct_range [ 585.667257] ok 12 addr [ 585.668399] ok 13 escaped_str [ 585.670212] ok 14 hex_string [ 585.671903] ok 15 mac [ 585.673389] ok 16 ip4 [ 585.674886] ok 17 ip6 [ 585.676255] ok 18 uuid [ 585.677875] ok 19 dentry [ 585.679138] ok 20 struct_va_format [ 585.679783] # time_and_date: EXPECTATION FAILED at lib/printf_kunit.c:97 vsnprintf(buf, 256, "%ptTsr", ...) wrote '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 585.680264] # time_and_date: EXPECTATION FAILED at lib/printf_kunit.c:135 kvasprintf(..., "%ptTsr", ...) returned '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 585.682436] not ok 21 time_and_date [ 585.683115] ok 22 struct_clk [ 585.685807] ok 23 bitmap [ 585.686576] ok 24 netdev_features [ 585.687243] ok 25 flags [ 585.687875] ok 26 errptr [ 585.688930] ok 27 fwnode_pointer [ 585.689544] ok 28 fourcc_pointer [ 585.689886] # printf: ran 448 tests [ 585.690215] # printf: pass:27 fail:1 skip:0 total:28 [ 585.690582] # Totals: pass:27 fail:1 skip:0 total:28 [ 585.691013] not ok 1 printf

I still have to look at the implementation before I add an ack.

And of course, I am also curious about what other reviewers think about it. And if this is OK for Rasmus.

Best Regards, Petr

Andy Shevchenko

4:01 p.m.

On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote:

...

On Mon 2025-02-10 13:23:21, Tamir Duberstein 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]

Signed-off-by: Tamir Duberstein tamird@gmail.com

I have just quickly tested this before leaving for a week. And I am fine with the result.

Seems reasonable to me. But I want a consensus with Rasmus.

-- With Best Regards, Andy Shevchenko

Tamir Duberstein

4:53 p.m.

On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:

...

On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote:

...
On Mon 2025-02-10 13:23:21, Tamir Duberstein 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]

Signed-off-by: Tamir Duberstein tamird@gmail.com

I have just quickly tested this before leaving for a week. And I am fine with the result.

Seems reasonable to me. But I want a consensus with Rasmus.

I have a local v4 where I've added the same enhancement as the scanf patches so that assertions log the line in the top-level test.

I'll wait for Rasmus' reply before sending. Tamir

Rasmus Villemoes

9:47 p.m.

On Fri, 14 Feb 2025 at 17:53, Tamir Duberstein tamird@gmail.com wrote:

...

On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:

...
On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote:

...

...
...
I have just quickly tested this before leaving for a week. And I am fine with the result.

Thanks, Petr, for demonstrating how it looks in a failure case.

...

...
Seems reasonable to me. But I want a consensus with Rasmus.

I have a local v4 where I've added the same enhancement as the scanf patches so that assertions log the line in the top-level test.

I'll wait for Rasmus' reply before sending.

I think all my concerns are addressed, with the lines printed in case of error telling what is wrong and not that memcmp() evaluating to 1 instead of 0, and with the final free-form comment including that "ran 448 tests". If you feel that word is confusing when there's "obviously" only 28 "test" being done, feel free to change that to "did 448 checks" or "did 448 individual checks" any other better wording.

Rasmus

Tamir Duberstein

9:51 p.m.

On Fri, Feb 14, 2025 at 4:47 PM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:

...

On Fri, 14 Feb 2025 at 17:53, Tamir Duberstein tamird@gmail.com wrote:

...
On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:

...
On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote:

...
...
...
I have just quickly tested this before leaving for a week. And I am fine with the result.

Thanks, Petr, for demonstrating how it looks in a failure case.

...
...
Seems reasonable to me. But I want a consensus with Rasmus.

I have a local v4 where I've added the same enhancement as the scanf patches so that assertions log the line in the top-level test.

I'll wait for Rasmus' reply before sending.

I think all my concerns are addressed, with the lines printed in case of error telling what is wrong and not that memcmp() evaluating to 1 instead of 0, and with the final free-form comment including that "ran 448 tests". If you feel that word is confusing when there's "obviously" only 28 "test" being done, feel free to change that to "did 448 checks" or "did 448 individual checks" any other better wording.

Rasmus

Personally, I don't feel strongly about this wording, so I'm hewing close to the original:

.... ok 25 flags ok 26 errptr ok 27 fwnode_pointer ok 28 fourcc_pointer # printf: ran 448 tests # printf: pass:28 fail:0 skip:0 total:28 # Totals: pass:28 fail:0 skip:0 total:28 ok 1 printf

I'll send v4 momentarily. Thanks, all!

days inactive

days old

linux-kselftest-mirror@lists.linaro.org

7 comments

participants

tags (0)

participants (4)

Andy Shevchenko
Petr Mladek
Rasmus Villemoes
Tamir Duberstein