Side note:
On Mon, Jun 6, 2016 at 2:28 AM, Masahiro Yamada
<[email protected]> 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").
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.
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
