On Mon, Sep 06, 2021 at 01:00:48PM -0700, Linus Torvalds wrote:
> So I just committed these three fixes:
>
> 4b93c544e90e ("thunderbolt: test: split up test cases in
> tb_test_credit_alloc_all")
> ba7b1f861086 ("lib/test_scanf: split up number parsing test routines")
> 1476ff21abb4 ("iwl: fix debug printf format strings")
>
> for the fallout from -Werror that I could easily check (mainly i386
> 'allyesconfig' - a situation I don't normally test).
>
> The printk format string one was trivial and I hopefully didn't screw
> anything up, but I'd ask people to look at and verify the two other
> ones. I tried to be very careful, and organizing the code movement in
> such a way that 'git diff' shows that it's doing the same thing before
> and after, but hey, mistakes happen.
>
> I found those two test-based ones somewhat annoying, because they both
> showed how little the test infrastructure tries to follow kernel
> rules. I bet those warnings have been showing up for a long long time,
> and people went "that's not a relevant configuration" or had some
> other reason to ignore them.
>
> No, the test cases may not be relevant in most situations, but it's
> not a good thing when something that is supposed to verify kernel
> behavior then violates some very fundamental and core kernel rules.
>
> And maybe it was simply missed. The one thing that was clear when I
> did that thunderbolt thing in particular is how easy it is to create
> variations of those 'struct some-assertion-struct' things on stack as
> part of the KUNIT infrastructure. That's unfortunate. It is possible
> that the solution to the kernel stack usage might have been to make
> those structures static instead, but I didn't check whether the
> description structs really can be.
Thanks for doing this! I certainly have received few mails from the
kbuildbot about this but haven't figured how to fix them properly.
Splitting the test to several small functions sounds like a good way to
do this. I'll keep this in mind in the future when adding more test
cases.
