Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 24 May 2008 13:49:56 +0300 | From | Adrian Bunk <> | Subject | Re: kernel coding style for if ... else which cross #ifdef |
| |
On Sat, May 24, 2008 at 11:06:21AM +0100, Jeremy Fitzhardinge wrote: > Sam Ravnborg wrote: >> On Fri, May 23, 2008 at 10:42:58PM -0700, H. Peter Anvin wrote: >> >>> Sam Ravnborg wrote: >>> >>>>> *However*, the best would really be if we changed Kconfig to emit >>>>> configuration constants what were 0/1 instead of >>>>> undefined/defined. That way we could do: >>>>> >>>>> if (CONFIG_SOMETHING && foo) { >>>>> /* ... something ... */ >>>>> } else if ((mode & S_IWUGO) == 0) { >>>>> /* ... */ >>>>> >>>> We could do that - but then it would need another >>>> name not to clash with all the places where we rely >>>> on CONFIG_FOO='n' => CONFIG_FOO is not defined. >>>> >>>> We could teach kconfig to emit something like: >>>> #define KFOO 0 (for the 'n' value) >>>> And 1 or 2 for the y and m values. >>>> >>>> >>> I don't think we want to use "1 or 2"... I suspect we want to use the >>> same booleans we currently have. >>> >> I'm a bit dense (or I need more coffe - it's morning here). >> What "same booleans"? >> > > They should be plain 0/1 booleans. For a bool/tristate option FOO, it > would define: > > Enabled y: > > #define CONFIG_FOO > #define CFG_FOO 1 > #undef CONFIG_FOO_MODULE > #define CFG_FOO_MODULE 0 > > > Enabled m: > > #define CONFIG_FOO > #define CFG_FOO 1 > #define CONFIG_FOO_MODULE > #define CFG_FOO_MODULE 1 >...
A quite common pattern in the kernel is: #if defined(CONFIG_FOO) || (defined(CONFIG_FOO_MODULE) && defined(MODULE))
Your suggestion would require them to be changed to: #if (defined(CONFIG_FOO) && !defined(CONFIG_FOO_MODULE)) || (defined(CONFIG_FOO_MODULE) && defined(MODULE))
(We could push these cases to kconfig, but there might also be other cases where changing the existing semantics of CONFIG_FOO could cause breakages.)
We see daily in kconfig that mixing tristates with bools is tricky (especially since bools are used with different intended semantics), and I don't think doing the same in the source files would be an improvement.
We might be able to do (without any CFG_FOO_MODULE at all):
Enabled m:
#undef CONFIG_FOO #define CFG_FOO 0 #define CONFIG_FOO_MODULE
And let everyone who want's "either y or m" semantics to convert the tristate to a bool with this semantics in kconfig himself:
config FOO tristate
config BAR def_bool FOO
Especially since this is actually a relatively unusual (and not nice) case since usually adding a module does not (and should) not change the kernel image.
> J
cu Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed
| ![\](/images/icornerr.gif) |