* [PATCH v4 1/3] printf: convert self-test to KUnit
2025-02-14 21:52 [PATCH v4 0/3] printf: convert self-test to KUnit Tamir Duberstein
@ 2025-02-14 21:52 ` Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 2/3] printf: break kunit into test cases Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 3/3] printf: implicate test line in failure messages Tamir Duberstein
2 siblings, 0 replies; 6+ messages in thread
From: Tamir Duberstein @ 2025-02-14 21:52 UTC (permalink / raw)
To: Arpitha Raghunandan, David Gow, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky,
Andrew Morton, Shuah Khan, Jonathan Corbet, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Brendan Higgins
Cc: linux-kernel, linux-kselftest, linux-doc, linux-m68k,
linuxppc-dev, Tamir Duberstein
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 92fc0eca7061..d85366e22ff6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25466,8 +25466,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 85b95d645b10..0c27745dff56 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
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 2/3] printf: break kunit into test cases
2025-02-14 21:52 [PATCH v4 0/3] printf: convert self-test to KUnit Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 1/3] " Tamir Duberstein
@ 2025-02-14 21:52 ` Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 3/3] printf: implicate test line in failure messages Tamir Duberstein
2 siblings, 0 replies; 6+ messages in thread
From: Tamir Duberstein @ 2025-02-14 21:52 UTC (permalink / raw)
To: Arpitha Raghunandan, David Gow, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky,
Andrew Morton, Shuah Khan, Jonathan Corbet, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Brendan Higgins
Cc: linux-kernel, linux-kselftest, linux-doc, linux-m68k,
linuxppc-dev, Tamir Duberstein
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),
{}
};
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 3/3] printf: implicate test line in failure messages
2025-02-14 21:52 [PATCH v4 0/3] printf: convert self-test to KUnit Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 1/3] " Tamir Duberstein
2025-02-14 21:52 ` [PATCH v4 2/3] printf: break kunit into test cases Tamir Duberstein
@ 2025-02-14 21:52 ` Tamir Duberstein
2025-02-19 20:41 ` Rasmus Villemoes
2 siblings, 1 reply; 6+ messages in thread
From: Tamir Duberstein @ 2025-02-14 21:52 UTC (permalink / raw)
To: Arpitha Raghunandan, David Gow, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky,
Andrew Morton, Shuah Khan, Jonathan Corbet, Geert Uytterhoeven,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Brendan Higgins
Cc: linux-kernel, linux-kselftest, linux-doc, linux-m68k,
linuxppc-dev, Tamir Duberstein
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 | 60 ++++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index 013df6f6dd49..aa5011156788 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -39,8 +39,8 @@ 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)
+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
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4 3/3] printf: implicate test line in failure messages
2025-02-14 21:52 ` [PATCH v4 3/3] printf: implicate test line in failure messages Tamir Duberstein
@ 2025-02-19 20:41 ` Rasmus Villemoes
2025-02-19 20:58 ` Tamir Duberstein
0 siblings, 1 reply; 6+ messages in thread
From: Rasmus Villemoes @ 2025-02-19 20:41 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Arpitha Raghunandan, David Gow, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Sergey Senozhatsky, Andrew Morton, Shuah Khan,
Jonathan Corbet, Geert Uytterhoeven, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Brendan Higgins, linux-kernel, linux-kselftest, linux-doc,
linux-m68k, linuxppc-dev
On Fri, 14 Feb 2025 at 22:53, Tamir Duberstein <tamird@gmail.com> 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 '
>
Actually, I'm not sure that is an improvement as-is, with the two
different line numbers being printed. It takes some thought to
recognize which one is relevant and which one is not.
Can't we have a variant of KUNIT_FAIL that allows one to pass the
file/line info when the caller has better info than the location of
the KUNIT_FAIL itself?
> static void __printf(5, 0)
> -do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen,
> - const char *fmt, va_list ap)
> +do_test(struct kunit *kunittest, const char *file, const int line, int bufsize, const char *expect,
> + int elen, const char *fmt, va_list ap)
This can't be right, the __printf attribute must be updated accordingly.
Rasmus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/3] printf: implicate test line in failure messages
2025-02-19 20:41 ` Rasmus Villemoes
@ 2025-02-19 20:58 ` Tamir Duberstein
0 siblings, 0 replies; 6+ messages in thread
From: Tamir Duberstein @ 2025-02-19 20:58 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Arpitha Raghunandan, David Gow, Petr Mladek, Steven Rostedt,
Andy Shevchenko, Sergey Senozhatsky, Andrew Morton, Shuah Khan,
Jonathan Corbet, Geert Uytterhoeven, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Brendan Higgins, linux-kernel, linux-kselftest, linux-doc,
linux-m68k, linuxppc-dev
On Wed, Feb 19, 2025 at 3:41 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On Fri, 14 Feb 2025 at 22:53, Tamir Duberstein <tamird@gmail.com> 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 '
> >
>
> Actually, I'm not sure that is an improvement as-is, with the two
> different line numbers being printed. It takes some thought to
> recognize which one is relevant and which one is not.
They're both relevant -- `do_test` does a bunch of checks, so you want
to know which particular check failed, but also which invocation at
the top level resulted in that check failing.
> Can't we have a variant of KUNIT_FAIL that allows one to pass the
> file/line info when the caller has better info than the location of
> the KUNIT_FAIL itself?
>
> > static void __printf(5, 0)
> > -do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen,
> > - const char *fmt, va_list ap)
> > +do_test(struct kunit *kunittest, const char *file, const int line, int bufsize, const char *expect,
> > + int elen, const char *fmt, va_list ap)
>
> This can't be right, the __printf attribute must be updated accordingly.
Good catch. Oddly I'm not able to reproduce any compiler complaints
here, even with the attribute completely removed.
^ permalink raw reply [flat|nested] 6+ messages in thread