linux-kernel - Re: [PATCH 2/2] lib/test: convert lib/test_list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABVgOS=3OcPK-s_PO9t87_ruaUq4d+AO0zuCC5SxRd3vC3Vxgg@mail.gmail.com>
Date:   Thu, 22 Apr 2021 13:52:17 +0800
From:   David Gow <davidgow@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 2/2] lib/test: convert lib/test_list_sort.c to use KUnit

On Thu, Apr 22, 2021 at 2:32 AM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> Functionally, this just means that the test output will be slightly
> changed and it'll now depend on CONFIG_KUNIT=y/m.
>
> It'll still run at boot time and can still be built as a loadable
> module.
>
> There was a pre-existing patch to convert this test that I found later,
> here [1]. Compared to [1], this patch doesn't rename files and uses
> KUnit features more heavily (i.e. does more than converting pr_err()
> calls to KUNIT_FAIL()).
>
> What this conversion gives us:
> * a shorter test thanks to KUnit's macros
> * a way to run this a bit more easily via kunit.py (and
> CONFIG_KUNIT_ALL_TESTS=y) [2]
> * a structured way of reporting pass/fail
> * uses kunit-managed allocations to avoid the risk of memory leaks
> * more descriptive error messages:
>   * i.e. it prints out which fields are invalid, what the expected
>   values are, etc.
>
> What this conversion does not do:
> * change the name of the file (and thus the name of the module)
> * change the name of the config option
>
> Leaving these as-is for now to minimize the impact to people wanting to
> run this test. IMO, that concern trumps following KUnit's style guide
> for both names, at least for now.
>
> [1] https://lore.kernel.org/linux-kselftest/20201015014616.309000-1-vitor@massaru.org/
> [2] Can be run via
> $ ./tools/testing/kunit/kunit.py run --kunitconfig /dev/stdin <<EOF
> CONFIG_KUNIT=y
> CONFIG_TEST_LIST_SORT=y
> EOF
>
> [16:55:56] Configuring KUnit Kernel ...
> [16:55:56] Building KUnit Kernel ...
> [16:56:29] Starting KUnit Kernel ...
> [16:56:32] ============================================================
> [16:56:32] ======== [PASSED] list_sort ========
> [16:56:32] [PASSED] list_sort_test
> [16:56:32] ============================================================
> [16:56:32] Testing complete. 1 tests run. 0 failed. 0 crashed.
> [16:56:32] Elapsed time: 35.668s total, 0.001s configuring, 32.725s building, 0.000s running
>
> Note: the build time is as after a `make mrproper`.
>
> Signed-off-by: Daniel Latypov <dlatypov@...gle.com>
> ---

This looks good to me: I'm not an expert in the test, though, so I may
have missed something. I did run it, though, and it seemed to work
fine.

It's a shame the functions can no-longer be marked __init, but I think
the advantages of KUnit outweigh it, particularly since this is
unlikely to be being used in production.

(BTW: This doesn't appear to be posted as a reply to Patch 1/2, which
made it a bit trickier to find.)

This is
Tested-by: David Gow <davidgow@...gle.com>

-- David