kconfig: Only generate config_is_xxx for bool and tristate options

Commit Message

Sam Ravnborg July 13, 2011, 8:08 p.m. UTC

On Wed, Jul 13, 2011 at 03:22:45PM +0200, Michal Marek wrote:
> On 18.5.2011 08:23, Arnaud Lacombe wrote:
>> Hi,
>>
>> [added [e-mail] to CC:]
>>
>> On Tue, May 17, 2011 at 3:53 PM, Sam Ravnborg<[e-mail]>  wrote:
>>> On Tue, May 17, 2011 at 05:35:32PM +0200, Michal Marek wrote:
>>>> For strings and integers, the config_is_xxx macros are useless and
>>>> sometimes misleading:
>>>>
>>>>    #define CONFIG_INITRAMFS_SOURCE ""
>>>>    #define config_is_initramfs_source() 1
>>>
>>> I'm late with this comment....
>>> Could we introduce "config_is_foo" using a syntax that
>>> does not break grepability?
>>>
>>> Maybe a syntax like this?
>>>
>>>     #ifdef CONFIG_FOO
>>>
>>> and
>>>
>>>     if (KCONFIG_FOO())
>>>
>>> Grepping for the use of a symbol is a very typical thing,
>>> so we should try to keep this.
>>> And with the suggested syntax above I expect fixdep to
>>> catch up both usage types.
>>>
>> Actually, there is already an issue, on a much smaller scale, in the
>> current tree with NUMA_BUILD and COMPACTION_BUILD. The real way to fix
>> this would be to always defines CONFIG_FOO, its value being 1 or 0
>> depending on whether or not the symbol is selected. This is a
>> +35k/-35k change.
>>
>> Also, I find KCONFIG_FOO() is too specific to be in the kernel source,
>> and redundant with CONFIG_FOO.
>>
>> I've been playing a bit with the preprocessor, and reached that point:
>>
>> #define EXPAND(x)       __ ## x
>> #define CONFIGURED(x)   \
>>          ({  int __1 __maybe_unused = 1;         \
>>              int __ ## x __maybe_unused = 0;     \
>>              EXPAND(x); })
>>
>> I am not specifically proud of that, use case would be:
>>
>> extern func(void);
>> int fn()
>> {
>>          if(CONFIGURED(CONFIG_FOO))
>>                  func();
>> }
>
> I finally got round to revisit this. Your approach inspired me to a much  
> simpler scheme: Instead of generating the config_is_xxx macros for  
> direct use in the code, name them __enabled_CONFIG_XXX or similar and  
> have a macro that expands given CONFIG_XXX symbol to the other form:

But then we clutter the namesapce with another set of variables that will
be misued. If not in the kernel then by other kconfig users.

Arnaud approach is nice as it does not require any additional
symbols to be generated.
We can always discuss tha naming - but the approach looks good.

To look at actual usage first:

int foo()
{
	if (KCONFIG(CONFIG_FOO)) {
		// this should be true for both module and non-module case
	}

	if (KCONFIG_NOT_MODULE(CONFIG_FOO)) {
		// this should be true only if this is not a module
	}



	if (KCONFIG_MODULE(CONFIG_FOO)) {
		// this should only be true for the module case
	}

}

Using the above even the naive reader is aware this is a kconfig generate
symbol.
Using "KCONFIG" in the name gives a stong hint that this is a kconfig thing.

We could stuff it all into a kconfig.h header like the attached (not tested)
but based on Arnauds sketch above - so creadits goes to him.

	Sam

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Michal Marek July 19, 2011, 1:45 p.m. UTC | #1

On 13.7.2011 22:08, Sam Ravnborg wrote:
> On Wed, Jul 13, 2011 at 03:22:45PM +0200, Michal Marek wrote:
>> On 18.5.2011 08:23, Arnaud Lacombe wrote:
>>> I've been playing a bit with the preprocessor, and reached that point:
>>>
>>> #define EXPAND(x)       __ ## x
>>> #define CONFIGURED(x)   \
>>>           ({  int __1 __maybe_unused = 1;         \
>>>               int __ ## x __maybe_unused = 0;     \
>>>               EXPAND(x); })
>>>
>>> I am not specifically proud of that, use case would be:
>>>
>>> extern func(void);
>>> int fn()
>>> {
>>>           if(CONFIGURED(CONFIG_FOO))
>>>                   func();
>>> }
>>
>> I finally got round to revisit this. Your approach inspired me to a much
>> simpler scheme: Instead of generating the config_is_xxx macros for
>> direct use in the code, name them __enabled_CONFIG_XXX or similar and
>> have a macro that expands given CONFIG_XXX symbol to the other form:
>
> But then we clutter the namesapce with another set of variables that will
> be misued. If not in the kernel then by other kconfig users.

Well, no sane person will prefix their variables with 
"__enabled_CONFIG_", so I'm not so worried about the namespace pollution.


> Arnaud approach is nice as it does not require any additional
> symbols to be generated.

Sure, I would also prefer a solution without that doesn't require 
generating new macros. But your's and Arnaud's trick doesn't eliminate 
the configured-out code at -O0, while with the help of the generated 
helper macros, the compiler sees an if(0) or if(1).


> +#include<linux/compiler.h>
> +
> +#define __symbol_value(sym)       __ ## x
> +#define __symbol_module(sym)      __symbol_value(__ ## sym ## _MODULE)
> +
> +/* return 1 if x is defined and not a module */
> +#define KCONFIG_NON_MODULE(sym)					\
> +	({ int __1 __maybe_unused = 1;				\
> +	   int __ ## sym __maybe_unused = 0;			\
> +	   __symbol_value(sym); })
> +
> +/* return 1 if sym is a module symbol */
> +#define KCONFIG_MODULE(sym)					\
> +	({ int __1 __maybe_unused = 1;				\
> +	   int __ ## sym ## _MODULE __maybe_unused = 0;	\
> +	   __symbol_value(sym); })
> +
> +/* return 1 if sym is defined - module or not */
> +#define KCONFIG(sym) (KCONFIG_NON_MODULE(sym) || KCONFIG_MODULE(sym))

With this extra expansion, the passed CONFIG_FOO expands to 1 here and 
the KCONFIG(_NON)_MODULE macros will then only see the 1 and fail.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
new file mode 100644
index 0000000..3ccdd73
--- /dev/null
+++ b/include/linux/kconfig.h
@@ -0,0 +1,30 @@ 
+#ifndef _LINUX_KCONFIG_H
+#define _LINUX_KCONFIG_H
+
+/*
+ * Helper macros for dealing with kconfig symbols
+ */
+
+#include <linux/compiler.h>
+
+#define __symbol_value(sym)       __ ## x
+#define __symbol_module(sym)      __symbol_value(__ ## sym ## _MODULE)
+
+/* return 1 if x is defined and not a module */
+#define KCONFIG_NON_MODULE(sym)					\
+	({ int __1 __maybe_unused = 1;				\
+	   int __ ## sym __maybe_unused = 0;			\
+	   __symbol_value(sym); })
+
+/* return 1 if sym is a module symbol */
+#define KCONFIG_MODULE(sym)					\
+	({ int __1 __maybe_unused = 1;				\
+	   int __ ## sym ## _MODULE __maybe_unused = 0;	\
+	   __symbol_value(sym); })
+
+/* return 1 if sym is defined - module or not */
+#define KCONFIG(sym) (KCONFIG_NON_MODULE(sym) || KCONFIG_MODULE(sym))
+
+#undef __symbol_value
+#undef __symbol_module
+#endif