New CONFIG_ guard: IF_FEATURE_BLAH(stuff)

Paul Fox pgf at brightstareng.com
Wed Feb 8 21:59:36 UTC 2006


 > 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.

 >   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":

     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) );

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

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.

paul
=---------------------
 paul fox, pgf at brightstareng.com



More information about the busybox mailing list