[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD3Xx4K5hg-sZb5Ek=q=cgPt59+Y8wgAs1iEWOFx2EMcz3EZAg@mail.gmail.com>
Date: Mon, 29 Sep 2014 22:25:22 +0200
From: Valentin Rothberg <valentinrothberg@...il.com>
To: Paul Bolle <pebolle@...cali.nl>
Cc: Kees Cook <keescook@...omium.org>,
Randy Dunlap <rdunlap@...radead.org>,
akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lib: rename TEST_MODULE to TEST_LKM
On Mon, Sep 29, 2014 at 10:18 PM, Paul Bolle <pebolle@...cali.nl> wrote:
> [Perhaps Kees hasn't seen this yet.]
>
> On Tue, 2014-09-23 at 20:27 +0200, Valentin Rothberg wrote:
>> On mar., 2014-09-23 at 10:11 -0700, Randy Dunlap wrote:
>> > On 09/23/14 10:10, Valentin Rothberg wrote:
>> > > On mar., 2014-09-23 at 09:49 -0700, Randy Dunlap wrote:
>> > >> On 09/22/14 23:58, Valentin Rothberg wrote:
>> > >>> The "_MODULE" suffix is reserved for tristates compiled as
>> > >>> loadable kernel modules (LKM). The "TEST_MODULE" feature thereby
>> > >>
>> > >> Is that documented anywhere?
>> > >
>> > > Sadly this is not made explicit, but the Kconfig code documents it. The
>> > > following code (./scripts/kconfig/confdata.c) is used to generate the
>> > > autoconf.h header file during the build process. When a feature is
>> > > selected as a kernel module ('m'), it is suffixed with "_MODULE" to
>> > > indicate it.
>> > >
>> > > » » switch (*value) {
>> > > » » case 'n':
>> > > » » » break;
>> > > » » case 'm':
>> > > » » » suffix = "_MODULE";
>> > > » » » /* fall through */
>> > >
>> > >> Was this causing some kind of problem or error? Please tell us what that was if so.
>> > >
>> > > It causes problems for static code analysis, which assumes a consistent
>> > > use of the "_MODULE" suffix.
>> > >
>> > > Another possible change would be to rename the reference in the Makefile
>> > > to "TEST_MODULE_MODULE". Personally, I prefer my proposed patch.
>> >
>> > Sure, your patch is fine, but we need to know *why* the patch is needed.
>>
>> Thank you for pointing that out. I will take care to give more
>> information in future patches.
>>
>> Thanks,
>> Valentin
>>
>> > Thanks.
>> >
>> > >>
>> > >>
>> > >>> violates this convention. The feature is used to compile the
>> > >>> lib/test_module.c kernel module.
>> > >>>
>> > >>> This patch renames the feature and its reference in a Makefile
>> > >>> to "TEST_LKM", which still expresses the test of a LKM.
>
> Maybe something like "TEST_MODULE_LOAD"?
It has been merged already, see [1]. Sorry, I did not check if you
were in the CC to let you know.
[1] http://ozlabs.org/~akpm/mmotm/broken-out/lib-rename-test_module-to-test_lkm.patch
>
>> > >>> Signed-off-by: Valentin Rothberg <valentinrothberg@...il.com>
>> > >>> ---
>> > >>> lib/Kconfig.debug | 2 +-
>> > >>> lib/Makefile | 2 +-
>> > >>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> > >>>
>> > >>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> > >>> index 07c2832..db91c97 100644
>> > >>> --- a/lib/Kconfig.debug
>> > >>> +++ b/lib/Kconfig.debug
>> > >>> @@ -1627,7 +1627,7 @@ config DMA_API_DEBUG
>> > >>>
>> > >>> If unsure, say N.
>> > >>>
>> > >>> -config TEST_MODULE
>> > >>> +config TEST_LKM
>> > >>> tristate "Test module loading with 'hello world' module"
>> > >>> default n
>> > >>> depends on m
>> > >>> diff --git a/lib/Makefile b/lib/Makefile
>> > >>> index d6b4bc4..382437a 100644
>> > >>> --- a/lib/Makefile
>> > >>> +++ b/lib/Makefile
>> > >>> @@ -31,7 +31,7 @@ obj-y += string_helpers.o
>> > >>> obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
>> > >>> obj-y += kstrtox.o
>> > >>> obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
>> > >>> -obj-$(CONFIG_TEST_MODULE) += test_module.o
>> > >>> +obj-$(CONFIG_TEST_LKM) += test_module.o
>
> Perhaps we should rename that file likewise?
>
>> > >>> obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
>> > >>> obj-$(CONFIG_TEST_BPF) += test_bpf.o
>> > >>> obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
>
> If something like this gets applied a follow up patch to make the kbuild
> system reject Kconfig symbols ending in _MODULE might be nice.
That is a very nice idea. Please CC me if you come up with a path.
Thanks,
Valentin
>
>
> Paul Bolle
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists