linux-hardening - Re: [PATCH] Documentation: KUnit: Update filename best practices

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] [day] [month] [year] [list]

Message-ID: <CABVgOSnUzuS6WDdZfX+1S1G7eotxtJWyvL_QmOMRuga1kOpViw@mail.gmail.com>
Date: Thu, 18 Jul 2024 13:56:51 +0800
From: David Gow <davidgow@...gle.com>
To: Kees Cook <kees@...nel.org>
Cc: Brendan Higgins <brendan.higgins@...ux.dev>, Rae Moar <rmoar@...gle.com>, 
	Jonathan Corbet <corbet@....net>, Linus Torvalds <torvalds@...ux-foundation.org>, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH] Documentation: KUnit: Update filename best practices

On Thu, 18 Jul 2024 at 05:00, Kees Cook <kees@...nel.org> wrote:
>
> Based on feedback from Linus[1], change the suggested file naming for
> KUnit tests.
>
> Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk=OHog@mail.gmail.com/ [1]
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> Cc: David Gow <davidgow@...gle.com>
> Cc: Brendan Higgins <brendan.higgins@...ux.dev>
> Cc: Rae Moar <rmoar@...gle.com>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: linux-kselftest@...r.kernel.org
> Cc: kunit-dev@...glegroups.com
> Cc: linux-doc@...r.kernel.org
> ---

Looks good to me. Maybe we could make it clearer that the suffix is
important for the module name, so if the module is made of multiple
source files, it will need to have the _test (or, now, _kunit) suffix
added in the Makefile.

Having the extra suffix on the module name shouldn't annoy modprobe as
much, as it doesn't need the file extension. So, e.g., if we have
foo_bar.ko and tests/foo_bar_kunit.ko:
- insmod doesn't have tab completion issues, as insmod foo[TAB] will
complete the filename to foo_bar.ko.
- modprobe also is fine, as modprobe foo[TAB] will complete the module
name partially to foo_bar (and adding _k[TAB] will complete to
foo_bar_kunit).

(It could still be annoying with fancy shells which show menus, or
something, but they'll be equally annoyed by all of the other options,
as far as I can tell.)

So:
- s/_test/_kunit for the default.
- Explicitly mention the module name, in addition to the filename.
Maybe also "if the module is made of multiple source files, specify
the module name (with the _kunit suffix) in the Makefile" or similar.

Cheers,
-- David


>  Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst
> index b6d0d7359f00..761dee3f89ca 100644
> --- a/Documentation/dev-tools/kunit/style.rst
> +++ b/Documentation/dev-tools/kunit/style.rst
> @@ -188,15 +188,20 @@ For example, a Kconfig entry might look like:
>  Test File and Module Names
>  ==========================
>
> -KUnit tests can often be compiled as a module. These modules should be named
> -after the test suite, followed by ``_test``. If this is likely to conflict with
> -non-KUnit tests, the suffix ``_kunit`` can also be used.
> -
> -The easiest way of achieving this is to name the file containing the test suite
> -``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be
> -placed next to the code under test.
> +Whether a KUnit test is compiled as a separate module or via an
> +``#include`` in a core kernel source file, the files should be named
> +after the test suite, followed by ``_test``, and live in a ``tests``
> +subdirectory to avoid conflicting with regular modules or the core kernel
> +source file names (e.g. for tab-completion). If this would conflict with
> +non-KUnit tests, the suffix ``_kunit`` can be used instead.
> +
> +So for the common case, name the file containing the test suite
> +``tests/<suite>_test.c`` (or, if needed, ``tests/<suite>_kunit.c``). The
> +``tests`` directory should be placed at the same level as the
> +code under test. For example, tests for ``lib/string.c`` live in
> +``lib/tests/string_test.c``.
>
>  If the suite name contains some or all of the name of the test's parent
>  directory, it may make sense to modify the source filename to reduce redundancy.
> -For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c``
> +For example, a ``foo_firmware`` suite could be in the ``tests/foo/firmware_test.c``
>  file.
> --
> 2.34.1
>