Re: [PATCH] kconfig: Only generate config_is_xxx for bool and tristate options

Michal Marek
2011-07-13T13:22:51Z

[-- Attachment #1: Type: text/plain, Size: 2538 bytes --]

On 18.5.2011 08:23, Arnaud Lacombe wrote:
> Hi,
>
> [added Valdis.Kletnieks@vt.edu to CC:]
>
> On Tue, May 17, 2011 at 3:53 PM, Sam Ravnborg<sam@ravnborg.org> 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:

/*
* Usage: ENABLED(CONFIG_FOO)
* Please do not use the __enabled_CONFIG_FOO defines directly to not break
* grepability of the code.
*/
#define ENABLED(x) __enabled_ ## x

plus a checkpatch.pl check so that people do not use the
__enabled_CONFIG_FOO macros in their code. git grep -w CONFIG_FOO
continues to work, fixdep continues to work, it works with -O0 because
it expands to a if(1) or if(0). Am I missing some obvious problem?

Attached is my test program:
$ gcc -Wall -O0 test.c
$ ./a.out
Foo1.0
Foo1.1
$ strings ./a.out | grep Foo
Foo1.0
Foo1.1

Michal

[-- Attachment #2: test.c --]
[-- Type: text/plain, Size: 520 bytes --]

#include <stdio.h>

#define CONFIG_FOO1 1
#undef CONFIG_FOO2
#define __enabled_CONFIG_FOO1 1
#define __enabled_CONFIG_FOO2 0

/*
* Usage: ENABLED(CONFIG_FOO)
* Please do not use the __enabled_CONFIG_FOO defines directly to not break
* grepability of the code.
*/
#define ENABLED(x) __enabled_ ## x

int main(void)
{
#ifdef CONFIG_FOO1
puts("Foo1.0");
#endif
#ifdef CONFIG_FOO2
puts("Foo2.0");
#endif
if (ENABLED(CONFIG_FOO1)) {
puts("Foo1.1");
}
if (ENABLED(CONFIG_FOO2)) {
puts("Foo2.1");
}
return 0;
}