lkml.org 
[lkml]   [2011]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] kconfig: Only generate config_is_xxx for bool and tristate options
    From
    Hi,

    On Wed, Jul 13, 2011 at 9:22 AM, Michal Marek <mmarek@suse.cz> wrote:
    > 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?
    >
    not I see immediately. ENABLED() will conflict with existing keyword
    in the tree, so it might need tweaking.

    Basically, you are taking the approach of always defining CONFIG_FOO
    (to 0 or 1), but by introducing a new macro, you avoid to break #ifdef
    usage in the tree.

    Actually, with this approach, we can even see forward and start replacing:

    #ifdef CONFIG_FOO

    by

    #if ENABLED(CONFIG_FOO)

    In a couple of release, we mark the old #ifdef syntax as deprecated,
    then after a couple of release, get rid of the duplicated macro
    altogether.

    You evil ! :-)

    - Arnaud

    > 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
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-07-13 17:23    [W:0.030 / U:30.864 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site