For strings and integers, the config_is_xxx macros are useless and
sometimes misleading:
#define CONFIG_INITRAMFS_SOURCE ""
#define config_is_initramfs_source() 1
Cc: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Signed-off-by: Michal Marek <[email protected]>
---
scripts/kconfig/confdata.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index db06af0..d40195d 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -808,7 +808,6 @@ int conf_write_autoconf(void)
const char *name;
FILE *out, *tristate, *out_h;
int i;
- int fct_val;
sym_clear_all_valid();
@@ -849,7 +848,7 @@ int conf_write_autoconf(void)
rootmenu.prompt->text);
for_all_symbols(i, sym) {
- fct_val = 1;
+ int fct_val = 0;
sym_calc_value(sym);
if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
continue;
@@ -863,7 +862,6 @@ int conf_write_autoconf(void)
case S_TRISTATE:
switch (sym_get_tristate_value(sym)) {
case no:
- fct_val = 0;
break;
case mod:
fprintf(tristate, "%s%s=M\n",
@@ -878,8 +876,10 @@ int conf_write_autoconf(void)
CONFIG_, sym->name);
fprintf(out_h, "#define %s%s 1\n",
CONFIG_, sym->name);
+ fct_val = 1;
break;
}
+ conf_write_function_autoconf(out_h, CONFIG_, sym->name, fct_val);
break;
case S_STRING:
conf_write_string(true, sym->name, sym_get_string_value(sym), out_h);
@@ -897,10 +897,8 @@ int conf_write_autoconf(void)
CONFIG_, sym->name, str);
break;
default:
- fct_val = 0;
break;
}
- conf_write_function_autoconf(out_h, CONFIG_, sym->name, fct_val);
}
fclose(out);
fclose(tristate);
--
1.7.4.1
On 17:35 Tue 17 May , Michal Marek wrote:
> For strings and integers, the config_is_xxx macros are useless and
> sometimes misleading:
except if the interger or hex can be at 0
so the config_is_ is usefull to known that it's enabled
>
> #define CONFIG_INITRAMFS_SOURCE ""
> #define config_is_initramfs_source() 1
here agreed but I'm nor sure if it's a special case or if it will we the case
for most of the string
Best Regards,
J.
On 17.5.2011 20:05, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:35 Tue 17 May , Michal Marek wrote:
>> For strings and integers, the config_is_xxx macros are useless and
>> sometimes misleading:
> except if the interger or hex can be at 0
> so the config_is_ is usefull to known that it's enabled
You can check if the option that the integer depends on is enabled.
>> #define CONFIG_INITRAMFS_SOURCE ""
>> #define config_is_initramfs_source() 1
> here agreed but I'm nor sure if it's a special case or if it will we the case
> for most of the string
Same here.
Michal
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.
Sam
On 21:53 Tue 17 May , Sam Ravnborg 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.
I'll prefer kconfig_foo()
not uppercase
but if we use KCONFIG_FOO no need to touch fixdep
Best Regards,
J.
On Wed, May 18, 2011 at 07:16:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:53 Tue 17 May , Sam Ravnborg 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.
> I'll prefer kconfig_foo()
> not uppercase
>
> but if we use KCONFIG_FOO no need to touch fixdep
fixdep is easy to fix - albeit it may cost a bit of processing time.
wht I worry much more about is users that miss uses of CONFIG_ symbols, because
they do not show up in "git grep CONFIG_FOO".
Using "if (KCONFIG_FOO()) users are also awre this is a configuration
decided condition - which is nice to stand out.
Sam
Hi,
[added [email protected] to CC:]
On Tue, May 17, 2011 at 3:53 PM, Sam Ravnborg <[email protected]> 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();
}
The issue I am seeing is that it adds a dependency to the optimizer,
as gcc will not optimize the branch away at -O0.
The advantage is that it is not intrusive at all within kconfig.
- Arnaud
Hi,
On Wed, May 18, 2011 at 2:19 AM, Sam Ravnborg <[email protected]> wrote:
> On Wed, May 18, 2011 at 07:16:45AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 21:53 Tue 17 May ? ? , Sam Ravnborg 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.
>> I'll prefer kconfig_foo()
>> not uppercase
>>
>> but if we use KCONFIG_FOO no need to touch fixdep
>
> fixdep is easy to fix - albeit it may cost a bit of processing time.
> wht I worry much more about is users that miss uses of CONFIG_ symbols, because
> they do not show up in "git grep CONFIG_FOO".
>
agree.
> Using "if (KCONFIG_FOO()) users are also awre this is a configuration
> decided condition - which is nice to stand out.
>
this will not work if you do $(git -w CONFIG_FOO) to avoid getting all
kind of noise in your search.
- Arnaud
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
>
> Cc: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
> Signed-off-by: Michal Marek <[email protected]>
> ---
> scripts/kconfig/confdata.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
I applied this to kbuild-2.6.git#kconfig, but I won't be sending this
and the config_is_xxx patch to Linus now. First, I'm a bit in hurry and
there have been proposals for either a different name of the macro, or a
different implementation, second, the fixdep fix is not here, so we
wouldn't be able to make use of the feature anyway. So let's settle on a
final solution for 2.6.41 or 3.1 or whatever comes second next.
Michal