diff mbox series

[v3,2/3] kconfig: new macro IF_ENABLED()

Message ID 20230219113629.39142-3-heinrich.schuchardt@canonical.com
State Superseded
Delegated to: Tom Rini
Headers show
Series spl: allow loading via partition type GUID | expand

Commit Message

Heinrich Schuchardt Feb. 19, 2023, 11:36 a.m. UTC
We want to move from using #ifdef to using if in our code. A lot of our
code using #ifdef is structured like:

    #ifdef CONFIG_FOO
        fun(CONFIG_FOO_OPT);
    #endif

In Kconfig you will find

    config FOO
        bool "enable foo"

    config FOO_OPT
        string "value for foo"
        depends on FOO

We cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not
defined.

     if (IS_ENABLED(CONFIG_FOO)
         fun(CONFIG_FOO_OPT);

This would result in an error due to the undefined symbol CONFIG_FOO_OPT.

The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates
to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to
the rescue:

     if (IS_ENABLED(CONFIG_FOO)
         fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");

If CONFIG_FOO is not defined, the compiler will see

    if (0)
        fun("");

and be happy.

If CONFIG_FOO is defined, the compiler will see

    if (1)
        fun(CONFIG_FOO_OPT)

and be equally happy.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v3:
	new patch
---
 include/linux/kconfig.h | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Andre Przywara Feb. 20, 2023, 5:14 p.m. UTC | #1
On Sun, 19 Feb 2023 12:36:28 +0100
Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote:

Hi,

> We want to move from using #ifdef to using if in our code. A lot of our
> code using #ifdef is structured like:
> 
>     #ifdef CONFIG_FOO
>         fun(CONFIG_FOO_OPT);
>     #endif
> 
> In Kconfig you will find
> 
>     config FOO
>         bool "enable foo"
> 
>     config FOO_OPT
>         string "value for foo"
>         depends on FOO
> 
> We cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not
> defined.
> 
>      if (IS_ENABLED(CONFIG_FOO)
>          fun(CONFIG_FOO_OPT);
> 
> This would result in an error due to the undefined symbol CONFIG_FOO_OPT.
> 
> The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates
> to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to
> the rescue:
> 
>      if (IS_ENABLED(CONFIG_FOO)
>          fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");

Ah, yeah, I like this one, this is indeed the most common pattern that
prevents many "#if to if" conversions.
I briefly thought about unifying those two lines, but it's probably
limiting, as it appears more flexible this way:

> 
> If CONFIG_FOO is not defined, the compiler will see
> 
>     if (0)
>         fun("");
> 
> and be happy.
> 
> If CONFIG_FOO is defined, the compiler will see
> 
>     if (1)
>         fun(CONFIG_FOO_OPT)
> 
> and be equally happy.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
> v3:
> 	new patch
> ---
>  include/linux/kconfig.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index 2bc704e110..7fea72da1a 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -28,6 +28,13 @@
>   */
>  #define IS_ENABLED(option)	config_enabled(option, 0)
>  
> +/*
> + * IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if
> + * CONFIG_FOO is set to 'y' and to def_val otherwise.
> + */
> +#define IF_ENABLED(option, opt_cfg, def_val) \
> +	config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val)
> +
>  /*
>   * U-Boot add-on: Helper macros to reference to different macros (prefixed by
>   * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build
Simon Glass Feb. 21, 2023, 7:41 p.m. UTC | #2
Hi Heinrich,

On Mon, 20 Feb 2023 at 10:15, Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 19 Feb 2023 12:36:28 +0100
> Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote:
>
> Hi,
>
> > We want to move from using #ifdef to using if in our code. A lot of our
> > code using #ifdef is structured like:
> >
> >     #ifdef CONFIG_FOO
> >         fun(CONFIG_FOO_OPT);
> >     #endif
> >
> > In Kconfig you will find
> >
> >     config FOO
> >         bool "enable foo"
> >
> >     config FOO_OPT
> >         string "value for foo"
> >         depends on FOO
> >
> > We cannot use CONFIG_FOO_OPT in our code directly if CONFIG_FOO is not
> > defined.
> >
> >      if (IS_ENABLED(CONFIG_FOO)
> >          fun(CONFIG_FOO_OPT);
> >
> > This would result in an error due to the undefined symbol CONFIG_FOO_OPT.
> >
> > The new macro IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) which evaluates
> > to opt_cfg if CONFIG_FOO is set to 'y' and to def_val otherwise comes to
> > the rescue:
> >
> >      if (IS_ENABLED(CONFIG_FOO)
> >          fun(IF_ENABLED(CONFIG_FOO, CONFIG_FOO_OPT, "");
>
> Ah, yeah, I like this one, this is indeed the most common pattern that
> prevents many "#if to if" conversions.
> I briefly thought about unifying those two lines, but it's probably
> limiting, as it appears more flexible this way:
>
> >
> > If CONFIG_FOO is not defined, the compiler will see
> >
> >     if (0)
> >         fun("");
> >
> > and be happy.
> >
> > If CONFIG_FOO is defined, the compiler will see
> >
> >     if (1)
> >         fun(CONFIG_FOO_OPT)
> >
> > and be equally happy.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>
> Cheers,
> Andre
>
> > ---
> > v3:
> >       new patch
> > ---
> >  include/linux/kconfig.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> > index 2bc704e110..7fea72da1a 100644
> > --- a/include/linux/kconfig.h
> > +++ b/include/linux/kconfig.h
> > @@ -28,6 +28,13 @@
> >   */
> >  #define IS_ENABLED(option)   config_enabled(option, 0)
> >
> > +/*
> > + * IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if
> > + * CONFIG_FOO is set to 'y' and to def_val otherwise.
> > + */
> > +#define IF_ENABLED(option, opt_cfg, def_val) \
> > +     config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val)
> > +
> >  /*
> >   * U-Boot add-on: Helper macros to reference to different macros (prefixed by
> >   * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build
>

This looks good to me, but what should we do with the existing
IF_ENABLED_INT()? Perhaps we should drop that in favour of this one,
or do we need both?

Regards,
Simon
diff mbox series

Patch

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 2bc704e110..7fea72da1a 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -28,6 +28,13 @@ 
  */
 #define IS_ENABLED(option)	config_enabled(option, 0)
 
+/*
+ * IF_ENABLED(CONFIG_FOO, opt_cfg, def_val) evalutes to opt_cfg if
+ * CONFIG_FOO is set to 'y' and to def_val otherwise.
+ */
+#define IF_ENABLED(option, opt_cfg, def_val) \
+	config_opt_enabled(IS_ENABLED(option), opt_cfg, def_val)
+
 /*
  * U-Boot add-on: Helper macros to reference to different macros (prefixed by
  * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build