linux-kernel - Re: [PATCH v2 3/5] kunit: string-stream-test: Make it a separate module

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: <CA+GJov6NBkPUmPSW6ir1Z0Gc9gFXP6dP-GhnoQU7nCRW0yXTLQ@mail.gmail.com>
Date: Fri, 21 Jun 2024 17:07:16 -0400
From: Rae Moar <rmoar@...gle.com>
To: Ivan Orlov <ivan.orlov0322@...il.com>
Cc: brendan.higgins@...ux.dev, davidgow@...gle.com, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org
Subject: Re: [PATCH v2 3/5] kunit: string-stream-test: Make it a separate module

On Tue, Jun 18, 2024 at 1:03 PM Ivan Orlov <ivan.orlov0322@...il.com> wrote:
>
> Currently, the only way to build string-stream-test is by setting
> CONFIG_KUNIT_TEST=y. However, CONFIG_KUNIT_TEST is a config option for
> a different test (`kunit-test.c`).
>
> Introduce a new Kconfig entry in order to be able to build the
> string-stream-test test as a separate module. Import the KUnit namespace
> in the test so we could have string-stream functions accessible.
>
> Reviewed-by: David Gow <davidgow@...gle.com>
> Signed-off-by: Ivan Orlov <ivan.orlov0322@...il.com>

Hello!

This is looking good to me other than the module description as noted
by Jeff. That could be a separate patch since the rest of the series
is looking good.

There is the checkpatch warning on the module description. But as
David mentioned, the description looks ok to me. If there is a new
version of this patch, it may be worth trying to get rid of the
warning by lengthening the description.

But I am happy with this patch as is.

Reviewed-by: Rae Moar <rmoar@...gle.com>

Thanks!
-Rae

> ---
> V1 -> V2:
> - No changes
>
>  lib/kunit/Kconfig              | 8 ++++++++
>  lib/kunit/Makefile             | 2 +-
>  lib/kunit/string-stream-test.c | 2 ++
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 34d7242d526d..b0713c0f9265 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -45,6 +45,14 @@ config KUNIT_TEST
>           purposes by developers interested in testing that KUnit works as
>           expected.
>
> +config KUNIT_STRING_STREAM_TEST
> +       tristate "KUnit test for string-stream" if !KUNIT_ALL_TESTS
> +       default KUNIT_ALL_TESTS
> +       help
> +         Enables the KUnit test for the string-stream (C++ stream style string
> +         builder used in KUnit for building messages). For the string-stream
> +         implementation, see lib/kunit/string-stream.c.
> +
>  config KUNIT_EXAMPLE_TEST
>         tristate "Example test for KUnit" if !KUNIT_ALL_TESTS
>         default KUNIT_ALL_TESTS
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 30f6bbf04a4a..478beb536dc9 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -19,10 +19,10 @@ endif
>  obj-y +=                               hooks.o
>
>  obj-$(CONFIG_KUNIT_TEST) +=            kunit-test.o
> +obj-$(CONFIG_KUNIT_STRING_STREAM_TEST) += string-stream-test.o
>
>  # string-stream-test compiles built-in only.
>  ifeq ($(CONFIG_KUNIT_TEST),y)
> -obj-$(CONFIG_KUNIT_TEST) +=            string-stream-test.o
>  obj-$(CONFIG_KUNIT_TEST) +=            assert_test.o
>  endif
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 7511442ea98f..d03cac934e04 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -534,3 +534,5 @@ static struct kunit_suite string_stream_test_suite = {
>         .init = string_stream_test_init,
>  };
>  kunit_test_suites(&string_stream_test_suite);
> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>