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 |
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
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 --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
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(+)