Messages in this thread | | | From | Masahiro Yamada <> | Date | Tue, 7 Jun 2016 06:44:58 +0900 | 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@linux-foundation.org>: > Side note: > > On Mon, Jun 6, 2016 at 2:28 AM, Masahiro Yamada > <yamada.masahiro@socionext.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
| |