[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ-ks9mb5pz4KWN_N-wd0k932bfhSjQuFEsXDrBxj9+RHSNWGA@mail.gmail.com>
Date: Fri, 7 Mar 2025 11:25:04 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Petr Mladek <pmladek@...e.com>
Cc: Arpitha Raghunandan <98.arpi@...il.com>, David Gow <davidgow@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>, Sergey Senozhatsky <senozhatsky@...omium.org>,
Andrew Morton <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>,
Jonathan Corbet <corbet@....net>, Geert Uytterhoeven <geert@...ux-m68k.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>, Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>, Christophe Leroy <christophe.leroy@...roup.eu>,
Naveen N Rao <naveen@...nel.org>, Brendan Higgins <brendan.higgins@...ux.dev>,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-doc@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: crng init: was: Re: [PATCH v5 2/3] printf: break kunit into test cases
On Fri, Mar 7, 2025 at 11:18 AM Petr Mladek <pmladek@...e.com> wrote:
>
> On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
> > Move all tests into `printf_test_cases`. This gives us nicer output in
> > the event of a failure.
> >
> > Combine `plain_format` and `plain_hash` into `hash_pointer` since
> > they're testing the same scenario.
> >
> > --- a/lib/tests/printf_kunit.c
> > +++ b/lib/tests/printf_kunit.c
> > @@ -178,7 +179,7 @@ test_number(void)
> > }
> >
> > static void
> > -test_string(void)
> > +test_string(struct kunit *kunittest)
> > {
> > test("", "%s%.0s", "", "123");
> > test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
> > @@ -215,29 +216,6 @@ test_string(void)
> > #define ZEROS "00000000" /* hex 32 zero bits */
> > #define ONES "ffffffff" /* hex 32 one bits */
> >
> > -static int
> > -plain_format(void)
> > -{
> > - char buf[PLAIN_BUF_SIZE];
> > - int nchars;
> > -
> > - nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
> > -
> > - if (nchars != PTR_WIDTH)
> > - return -1;
> > -
> > - if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> > - kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
> > - PTR_VAL_NO_CRNG);
> > - return 0;
> > - }
> > -
> > - if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
> > - return -1;
> > -
> > - return 0;
> > -}
> > -
> > #else
> >
> > #define PTR_WIDTH 8
> > @@ -247,89 +225,44 @@ plain_format(void)
> > #define ZEROS ""
> > #define ONES ""
> >
> > -static int
> > -plain_format(void)
> > -{
> > - /* Format is implicitly tested for 32 bit machines by plain_hash() */
> > - return 0;
> > -}
> > -
> > #endif /* BITS_PER_LONG == 64 */
> >
> > -static int
> > -plain_hash_to_buffer(const void *p, char *buf, size_t len)
> > +static void
> > +plain_hash_to_buffer(struct kunit *kunittest, const void *p, char *buf, size_t len)
> > {
> > - int nchars;
> > -
> > - nchars = snprintf(buf, len, "%p", p);
> > -
> > - if (nchars != PTR_WIDTH)
> > - return -1;
> > + KUNIT_ASSERT_EQ(kunittest, snprintf(buf, len, "%p", p), PTR_WIDTH);
> >
> > if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> > kunit_warn(kunittest, "crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
> > PTR_VAL_NO_CRNG);
> > - return 0;
>
> I have simulated the not-yet-initialized crng and got:
>
> [ 80.109760] printf_kunit: module verification failed: signature and/or required key missing - tainting kernel
> [ 80.114218] KTAP version 1
> [ 80.114743] 1..1
> [ 80.116124] KTAP version 1
> [ 80.116752] # Subtest: printf
> [ 80.117239] # module: printf_kunit
> [ 80.117256] 1..28
> [ 80.120924] ok 1 test_basic
> [ 80.121495] ok 2 test_number
> [ 80.122741] ok 3 test_string
> [ 80.123498] # hash_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 80.124044] # hash_pointer: EXPECTATION FAILED at lib/tests/printf_kunit.c:256
> Expected buf == "00000000", but
> buf ==
> <28><5f><5f><5f><5f><70><74><72>
> "00000000" ==
> <30><30><30><30><30><30><30><30>
> [ 80.125888] not ok 4 hash_pointer
> [ 80.129831] ok 5 null_pointer
> [ 80.130253] ok 6 error_pointer
> [ 80.131221] # invalid_pointer: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 80.132168] ok 7 invalid_pointer
> [ 80.135149] ok 8 symbol_ptr
> [ 80.136016] ok 9 kernel_ptr
> [ 80.136868] ok 10 struct_resource
> [ 80.137768] ok 11 struct_range
> [ 80.138613] ok 12 addr
> [ 80.139370] ok 13 escaped_str
> [ 80.140054] ok 14 hex_string
> [ 80.140601] ok 15 mac
> [ 80.141162] ok 16 ip4
> [ 80.141670] ok 17 ip6
> [ 80.142221] ok 18 uuid
> [ 80.143090] ok 19 dentry
> [ 80.143963] ok 20 struct_va_format
> [ 80.144523] ok 21 time_and_date
> [ 80.145043] ok 22 struct_clk
> [ 80.145589] ok 23 bitmap
> [ 80.146087] ok 24 netdev_features
> [ 80.146572] ok 25 flags
> [ 80.146980] # errptr: crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 80.147412] ok 26 errptr
> [ 80.148548] ok 27 fwnode_pointer
> [ 80.149086] ok 28 fourcc_pointer
> [ 80.149090] # printf: ran 448 tests
> [ 80.149099] # printf: pass:27 fail:1 skip:0 total:28
> [ 80.149102] # Totals: pass:27 fail:1 skip:0 total:28
> [ 80.149106] not ok 1 printf
>
> => One test failed even though vspritf() worked as expected.
>
> The "EXPECTATION FAILED" message was a bit tricky because
> it printed "<28><5f><5f><5f><5f><70><74><72>" instead of "(____ptrval____)".
>
> Two tests succeeded even after a warning message which would make people
> to investigate it.
>
> I suggest to rather skip the test in this case. Something like:
>
> if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> kunit_skip(kunittest,
> "crng possibly not yet initialized. plain 'p' buffer contains \"%s\"\n",
> PTR_VAL_NO_CRNG);
> }
>
>
> It produces:
>
> [ 140.555055] KTAP version 1
> [ 140.555413] 1..1
> [ 140.555796] KTAP version 1
> [ 140.556115] # Subtest: printf
> [ 140.556450] # module: printf_kunit
> [ 140.556459] 1..28
> [ 140.557757] ok 1 test_basic
> [ 140.558072] ok 2 test_number
> [ 140.558693] ok 3 test_string
> [ 140.559278] ok 4 hash_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 140.560949] ok 5 null_pointer
> [ 140.561257] ok 6 error_pointer
> [ 140.561880] ok 7 invalid_pointer # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 140.564159] ok 8 symbol_ptr
> [ 140.565248] ok 9 kernel_ptr
> [ 140.566346] ok 10 struct_resource
> [ 140.567642] ok 11 struct_range
> [ 140.569141] ok 12 addr
> [ 140.570395] ok 13 escaped_str
> [ 140.571407] ok 14 hex_string
> [ 140.572337] ok 15 mac
> [ 140.573572] ok 16 ip4
> [ 140.574712] ok 17 ip6
> [ 140.575743] ok 18 uuid
> [ 140.577164] ok 19 dentry
> [ 140.578248] ok 20 struct_va_format
> [ 140.579400] ok 21 time_and_date
> [ 140.580507] ok 22 struct_clk
> [ 140.581706] ok 23 bitmap
> [ 140.582739] ok 24 netdev_features
> [ 140.583808] ok 25 flags
> [ 140.585274] ok 26 errptr # SKIP crng possibly not yet initialized. plain 'p' buffer contains "(____ptrval____)"
> [ 140.588403] ok 27 fwnode_pointer
> [ 140.592141] ok 28 fourcc_pointer
> [ 140.592758] # printf: ran 408 tests
> [ 140.593219] # printf: pass:25 fail:0 skip:3 total:28
> [ 140.593706] # Totals: pass:25 fail:0 skip:3 total:28
> [ 140.594280] ok 1 printf
>
> Best Regards,
> Petr
Thanks for testing! I didn't know how to do that. Changed to
kunit_skip as you suggested.
Powered by blists - more mailing lists