linux-kernel - Re: [PATCH] kconfig.h: use already defined macros for IS

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]

Date:	Tue, 7 Jun 2016 06:44:58 +0900
From:	Masahiro Yamada <yamada.masahiro@...ionext.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Simek <michal.simek@...inx.com>,
	Nicolas Pitre <nico@...aro.org>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Michal Marek <mmarek@...e.cz>, Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] kconfig.h: use already defined macros for IS_REACHABLE() define

Hi Linus,


2016-06-07 1:36 GMT+09:00 Linus Torvalds <torvalds@...ux-foundation.org>:
> Side note:
>
> On Mon, Jun 6, 2016 at 2:28 AM, Masahiro Yamada
> <yamada.masahiro@...ionext.com> wrote:
>>
>> -#define IS_REACHABLE(option) (config_enabled(option) || \
>> -                (config_enabled(option##_MODULE) && config_enabled(MODULE)))
>> +#define IS_REACHABLE(option) (IS_BUILTIN(option) || \
>> +                (IS_MODULE(option) && config_enabled(MODULE)))
>
> Is that "config_enabled(MODULE)" actually sensible?
>
> The whole "config_enabled()" thing is designed for config options. But
> "MODULE" is not a config option, it's per-file build option ("are we
> now building for a module" vs "are we building built-in code").

I thought of this, too.

Because config_enabled() is so useful,
maybe people tend to abuse it.


I see one case where config_enabled() is used
for a non-config macro.


#define __EXPORT_SYMBOL(sym, sec) \
           __cond_export_sym(sym, sec, config_enabled(__KSYM_##sym))

Assuming we can do something with that,
ultimately I'd like to ban the use of
config_enabled() outside of include/linux/kconfig.h

I already started this work:
http://www.spinics.net/lists/mips/msg63759.html




> The code clearly works, but it smells a bit confusing to me. Talking
> about "config" of MODULE makes me think CONFIG_MODULES ("are modular
> builds enabled") rather than "am I currently building a module".
>
> I wonder if we should have something like
>
>   #ifdef MODULE
>    #define BUILDING_MODULES 1
>   #else
>    #define BUILDING_MODULES 0
>   #endif
>
> and then using (IS_MODULE(option) && BUILDING_MODULES) to clarify the test.


MODULE is defined / undefined per file.

So, I think BUILDING_MODULE makes more sense than BUILDING_MODULES.



> Because when I first looked at the patch and didn't think about it any
> more, my initial reaction was "why is it checking whether modules are
> enabled - if IS_MODULE() is true, then _obviously_ modules are
> enabled?"
>
> But maybe that's just me.
>
>                Linus



-- 
Best Regards
Masahiro Yamada