New CONFIG_ guard: IF_FEATURE_BLAH(stuff)

[Rob Landley]

On Wednesday 08 February 2006 15:59, Paul Fox wrote:
>  > Suggestion:
>  >
>  > As long as we're generating the ENABLE_BLAH from the CONFIG_BLAH, we
>  > might as well generate IF_BLAH(x) that looks about like:
>  >
>  > #ifdef CONFIG_BLAH
>  > #define IF_BLAH(x) x
>  > #else
>  > #define IF_BLAH(x)
>  > #endif
>
> well, to me this starts feeling like over-macroization:  the
> obscurity it fixes is only barely worse than the obfuscation
> introduced.  :-)
>
> however, you're right that the need comes up quite a bit in busybox,
> so maybe its justified.

Configuring out chunks of the code is never going to be pretty, no matter how 
we do it.  Having more than one #ifdef in the middle of a function parameter 
list, and this being a regular occurrence, is something I'd rather like to 
avoid though.

>  >   opt = bb_getopt_ulflags(argc, argv, "Rs:ud:r:"
>  >  IF_FEATURE_DATE_ISOFMT("I::"),
>  >  &date_str, &date_str, &filename
>  >  IF_FEATURE_DATE_ISOFMT(,&isofmt_arg) );
>  >
>  > Opinions?
>
> if you do this, please at least consider other names, which might make
> the code more readable.  remember, i said "might":

It turns out we already have a USAGE_ macro that does almost what I want.  I'm 
happy to re-use the existing name if that's what's already there.

>      opt = bb_getopt_ulflags(argc, argv, "Rs:ud:r:"
>  ONLY_IF_FEATURE_DATE_ISOFMT("I::"),
>  &date_str, &date_str, &filename
>  ONLY_IF_FEATURE_DATE_ISOFMT(,&isofmt_arg) );

opt = bb_getopt_ulflags(argc, argv, "Rs:ud:r:"
 USAGE_FEATURE_DATE_ISOFMT("I::"),
 &date_str, &date_str, &filename
 USAGE_FEATURE_DATE_ISOFMT(,&isofmt_arg) );

The tricky bit is the second one, which has a comma in it, so with the naieve 
"#define USAGE_FEATURE_DATE_ISOFMT(x) x" the compiler barfs:
macro "USAGE_FEATURE_DATE_ISOFMT" passed 2 arguments, but takes just 1

So what you need is:
#define USAGE_FEATURE_DATE_ISOFMT(...) __VA_ARGS__

And _then_ it works right.  (Figuring that out took a while.)

I still want to merge this in with bb_config.h and make _usage.h go away.  I 
sent an email to vodz and hopefully he can fix up bb_depend.

> these suggestions are in the spirit of making it as obvious as
> possible what the macro does, without having to recheck the
> header file.  remember that many of us don't work with the
> busybox code on the daily (or hourly :-) basis that you do so.

One of my proposed OLS presentations is "Busybox: not just for breakfast 
anymore", and if they accept that proposal I'm going to use it as an excuse 
to document the heck out of everything I can think of.

And one thing I'm definitely documenting is the #ifdef reduction program, 
including the CONFIG->ENABLE migration and the USAGE() guards.  (That should 
go on the programming.html page anyway.)

> paul

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.