On Mon 2020-08-17 09:06:32, Rasmus Villemoes wrote: > On 17/08/2020 06.30, Arpitha Raghunandan wrote: > > Converts test lib/test_printf.c to KUnit. > > More information about KUnit can be found at > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > > KUnit provides a common framework for unit tests in the kernel. > > So I can continue to build a kernel with some appropriate CONFIG set to > y, boot it under virt-me, run dmesg and see if I broke printf? That's > what I do now, and I don't want to have to start using some enterprisy > framework. I had the same concern. I have tried it. It compiled a module called printf_kunit.c. #> modprobe printf_kunit produced the following in dmesg: [ 60.931175] printf_kunit: module verification failed: signature and/or required key missing - tainting kernel [ 60.942209] TAP version 14 [ 60.945197] # Subtest: printf-kunit-test [ 60.945200] 1..1 [ 60.951092] ok 1 - selftest [ 60.953414] ok 1 - printf-kunit-test I could live with the above. Then I tried to break a test by the following change: diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c index 68ac5f9b8d28..1689dadd70a3 100644 --- a/lib/printf_kunit.c +++ b/lib/printf_kunit.c @@ -395,7 +395,7 @@ ip4(struct kunit *kunittest) sa.sin_port = cpu_to_be16(12345); sa.sin_addr.s_addr = cpu_to_be32(0x7f000001); - test(kunittest, "127.000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); + test(kunittest, "127-000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); test(kunittest, "127.000.000.001|127.0.0.1", "%piS|%pIS", &sa, &sa); sa.sin_addr.s_addr = cpu_to_be32(0x01020304); test(kunittest, "001.002.003.004:12345|1.2.3.4:12345", "%piSp|%pISp", &sa, &sa); It produced:: [ 56.786858] TAP version 14 [ 56.787493] # Subtest: printf-kunit-test [ 56.787494] 1..1 [ 56.788612] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 Expected memcmp(test_buffer, expect, written) == 0, but memcmp(test_buffer, expect, written) == 1 0 == 0 vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' [ 56.795433] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 Expected memcmp(test_buffer, expect, written) == 0, but memcmp(test_buffer, expect, written) == 1 0 == 0 vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote '127.000.000.001|127', expected '127-000.000.001|127' [ 56.800909] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:108 Expected memcmp(p, expect, elen+1) == 0, but memcmp(p, expect, elen+1) == 1 0 == 0 kvasprintf(..., "%pi4|%pI4", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' [ 56.806497] not ok 1 - selftest [ 56.806497] not ok 1 - printf-kunit-test while the original code would have written the following error messages: [ 95.859225] test_printf: loaded. [ 95.860031] test_printf: vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' [ 95.862630] test_printf: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote '127.0', expected '127-0' [ 95.864118] test_printf: kvasprintf(..., "%pi4|%pI4", ...) returned '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' [ 95.866589] test_printf: failed 3 out of 388 tests Even the error output is acceptable for me. I am just curious why the 2nd failure is different: + original code: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote '127.0', expected '127-0' + kunit code: vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote '127.000.000.001|127', expected '127-000.000.001|127' I am also a bit scared by the following note at https://www.kernel.org/doc/html/latest/dev-tools/kunit/start.html#running-tests-without-the-kunit-wrapper "KUnit is not designed for use in a production system, and it’s possible that tests may reduce the stability or security of the system." What does it mean thay it might reduce stability or security? Is it because the tests might cause problems? Or because the kunit framework modifies functionality of the running system all the time? Best Regards, Petr PS: I still have to look at the code. Anwyay, the comments from Rasmus make sense.