diff mbox series

[06/11] kasan: rename CONFIG_TEST_KASAN_MODULE

Message ID ae666d8946f586cfc250205cea4ae0b729d818fa.1609871239.git.andreyknvl@google.com (mailing list archive)
State New, archived
Headers show
Series kasan: HW_TAGS tests support and fixes | expand

Commit Message

Andrey Konovalov Jan. 5, 2021, 6:27 p.m. UTC
Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.

This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f
---
 Documentation/dev-tools/kasan.rst | 6 +++---
 lib/Kconfig.kasan                 | 2 +-
 lib/Makefile                      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Alexander Potapenko Jan. 12, 2021, 8:09 a.m. UTC | #1
On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@google.com> wrote:
>
> Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.
>
> This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f
Reviewed-by: Alexander Potapenko <glider@google.com>


>  KASAN tests consist on two parts:

While at it: "consist of".
Marco Elver Jan. 12, 2021, 1:33 p.m. UTC | #2
On Tue, Jan 05, 2021 at 07:27PM +0100, Andrey Konovalov wrote:
> Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.
> 
> This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f

Reviewed-by: Marco Elver <elver@google.com>

For this patch, as-is. But we could potentially do better in future --
see below.

> ---
>  Documentation/dev-tools/kasan.rst | 6 +++---
>  lib/Kconfig.kasan                 | 2 +-
>  lib/Makefile                      | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index 26c99852a852..72535816145d 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code.
>  This allows ``VMAP_STACK`` support on x86, and can simplify support of
>  architectures that do not have a fixed module region.
>  
> -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE
> ---------------------------------------------------
> +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST
> +----------------------------------------------------
>  
>  KASAN tests consist on two parts:
>  
> @@ -384,7 +384,7 @@ KASAN tests consist on two parts:
>  automatically in a few different ways, see the instructions below.
>  
>  2. Tests that are currently incompatible with KUnit. Enabled with
> -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can
> +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can
>  only be verified manually, by loading the kernel module and inspecting the
>  kernel log for KASAN reports.
>  
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index 3091432acb0a..624ae1df7984 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST
>  	  For more information on KUnit and unit tests in general, please refer
>  	  to the KUnit documentation in Documentation/dev-tools/kunit.
>  
> -config TEST_KASAN_MODULE
> +config KASAN_MODULE_TEST
>  	tristate "KUnit-incompatible tests of KASAN bug detection capabilities"
>  	depends on m && KASAN && !KASAN_HW_TAGS
>  	help
> diff --git a/lib/Makefile b/lib/Makefile
> index afeff05fa8c5..122f25d6407e 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o
>  obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
>  CFLAGS_test_kasan.o += -fno-builtin
>  CFLAGS_test_kasan.o += $(call cc-disable-warning, vla)
> -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o
> +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o
>  CFLAGS_test_kasan_module.o += -fno-builtin

[1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names

Do we eventually want to rename the tests to follow the style
recommendation more closely?

Option 1: Rename the KUnit test to kasan_test.c? And then
also rename test_kasan_module.c -> kasan_module_test.c?  Then the file
names would be mostly consistent with the config names.

Option 2: The style guide [1] also mentions where there are non-KUnit
tests around to use _kunit for KUnit test, and _test (or similar) for
the non-KUnit test. So here we'd end up with kasan_kunit.c and
kasan_test.c. That would get rid of the confusing "module" part. The
config variable could either remain CONFIG_KASAN_MODULE_TEST, or simply
become CONFIG_KASAN_TEST, since we already have CONFIG_KASAN_KUNIT_TEST
to distinguish.

But I won't bikeshed further. If you do a v2, I leave it to your
judgement to decide what is most appropriate.

Thanks,
-- Marco
Andrey Konovalov Jan. 12, 2021, 6:26 p.m. UTC | #3
On Tue, Jan 12, 2021 at 9:10 AM Alexander Potapenko <glider@google.com> wrote:
>
> On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@google.com> wrote:
> >
> > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.
> >
> > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f
> Reviewed-by: Alexander Potapenko <glider@google.com>
>
>
> >  KASAN tests consist on two parts:
>
> While at it: "consist of".

Will do in v2, thanks!
Andrey Konovalov Jan. 12, 2021, 6:28 p.m. UTC | #4
On Tue, Jan 12, 2021 at 2:33 PM Marco Elver <elver@google.com> wrote:
>
> On Tue, Jan 05, 2021 at 07:27PM +0100, Andrey Konovalov wrote:
> > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.
> >
> > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f
>
> Reviewed-by: Marco Elver <elver@google.com>
>
> For this patch, as-is. But we could potentially do better in future --
> see below.
>
> > ---
> >  Documentation/dev-tools/kasan.rst | 6 +++---
> >  lib/Kconfig.kasan                 | 2 +-
> >  lib/Makefile                      | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> > index 26c99852a852..72535816145d 100644
> > --- a/Documentation/dev-tools/kasan.rst
> > +++ b/Documentation/dev-tools/kasan.rst
> > @@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code.
> >  This allows ``VMAP_STACK`` support on x86, and can simplify support of
> >  architectures that do not have a fixed module region.
> >
> > -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE
> > ---------------------------------------------------
> > +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST
> > +----------------------------------------------------
> >
> >  KASAN tests consist on two parts:
> >
> > @@ -384,7 +384,7 @@ KASAN tests consist on two parts:
> >  automatically in a few different ways, see the instructions below.
> >
> >  2. Tests that are currently incompatible with KUnit. Enabled with
> > -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can
> > +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can
> >  only be verified manually, by loading the kernel module and inspecting the
> >  kernel log for KASAN reports.
> >
> > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> > index 3091432acb0a..624ae1df7984 100644
> > --- a/lib/Kconfig.kasan
> > +++ b/lib/Kconfig.kasan
> > @@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST
> >         For more information on KUnit and unit tests in general, please refer
> >         to the KUnit documentation in Documentation/dev-tools/kunit.
> >
> > -config TEST_KASAN_MODULE
> > +config KASAN_MODULE_TEST
> >       tristate "KUnit-incompatible tests of KASAN bug detection capabilities"
> >       depends on m && KASAN && !KASAN_HW_TAGS
> >       help
> > diff --git a/lib/Makefile b/lib/Makefile
> > index afeff05fa8c5..122f25d6407e 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o
> >  obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
> >  CFLAGS_test_kasan.o += -fno-builtin
> >  CFLAGS_test_kasan.o += $(call cc-disable-warning, vla)
> > -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o
> > +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o
> >  CFLAGS_test_kasan_module.o += -fno-builtin
>
> [1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names
>
> Do we eventually want to rename the tests to follow the style
> recommendation more closely?
>
> Option 1: Rename the KUnit test to kasan_test.c? And then
> also rename test_kasan_module.c -> kasan_module_test.c?  Then the file
> names would be mostly consistent with the config names.
>
> Option 2: The style guide [1] also mentions where there are non-KUnit
> tests around to use _kunit for KUnit test, and _test (or similar) for
> the non-KUnit test. So here we'd end up with kasan_kunit.c and
> kasan_test.c. That would get rid of the confusing "module" part. The
> config variable could either remain CONFIG_KASAN_MODULE_TEST, or simply
> become CONFIG_KASAN_TEST, since we already have CONFIG_KASAN_KUNIT_TEST
> to distinguish.
>
> But I won't bikeshed further. If you do a v2, I leave it to your
> judgement to decide what is most appropriate.

Most tests in lib/ start with test_, so not using that pattern for
KASAN tests could be confusing. Maybe we can move them to mm/kasan.
Anyway, I won't look into this right now.

Thanks!
diff mbox series

Patch

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 26c99852a852..72535816145d 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -374,8 +374,8 @@  unmapped. This will require changes in arch-specific code.
 This allows ``VMAP_STACK`` support on x86, and can simplify support of
 architectures that do not have a fixed module region.
 
-CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE
---------------------------------------------------
+CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST
+----------------------------------------------------
 
 KASAN tests consist on two parts:
 
@@ -384,7 +384,7 @@  KASAN tests consist on two parts:
 automatically in a few different ways, see the instructions below.
 
 2. Tests that are currently incompatible with KUnit. Enabled with
-``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can
+``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can
 only be verified manually, by loading the kernel module and inspecting the
 kernel log for KASAN reports.
 
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 3091432acb0a..624ae1df7984 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -192,7 +192,7 @@  config KASAN_KUNIT_TEST
 	  For more information on KUnit and unit tests in general, please refer
 	  to the KUnit documentation in Documentation/dev-tools/kunit.
 
-config TEST_KASAN_MODULE
+config KASAN_MODULE_TEST
 	tristate "KUnit-incompatible tests of KASAN bug detection capabilities"
 	depends on m && KASAN && !KASAN_HW_TAGS
 	help
diff --git a/lib/Makefile b/lib/Makefile
index afeff05fa8c5..122f25d6407e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -68,7 +68,7 @@  obj-$(CONFIG_TEST_IDA) += test_ida.o
 obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
 CFLAGS_test_kasan.o += -fno-builtin
 CFLAGS_test_kasan.o += $(call cc-disable-warning, vla)
-obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o
+obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o
 CFLAGS_test_kasan_module.o += -fno-builtin
 obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o
 CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)