New CONFIG_ guard: IF_FEATURE_BLAH(stuff)

[Rob Landley]

On Wednesday 08 February 2006 17:47, Devin Bayer wrote:
> Rob Landley wrote:
> > #ifdef CONFIG_FEATURE_DATE_ISOFMT
> > # define GETOPT_ISOFMT  "I::"
> > #else
> > # define GETOPT_ISOFMT
> > #endif
> >     bb_opt_complementally = "?:d--s:s--d";
> >     opt = bb_getopt_ulflags(argc, argv, "Rs:ud:r:" GETOPT_ISOFMT,
> >                     &date_str, &date_str, &filename
> > #ifdef CONFIG_FEATURE_DATE_ISOFMT
> >                     , &isofmt_arg
> > #endif
> >                     );
>
> How about:
>
> opt = bb_getopt_ulflags(argc, argv, "Rs:ud:r:I::",
>       &date_str, &date_str, &filename, &isofmt_arg);
>
> And have "char *unimplemented" in bb_getopt_ulflags.c.
> Then at the top of the function do:
>
> #if ENABLE_FEATURE_DATE_ISOFMT
>  char *isofmt_arg;
> #else
>  #define isofmt_arg unimplemented
> #endif

This doesn't save us any space.  The call to bb_getopt_ulflags is still 
copying extra arguments onto the stack, generating the full-sized code.  The 
reason to configure features out is to make the resulting code smaller.

Plus, it's always recognizing command line arguments even when it doesn't 
support them.  When you've disabled support for SORT_BIG you want sort to 
complain if you try to use -k, not silently behave incorrectly.

> so when bb_getopt_ulflags sees an argument == &unimplemented, it'll spit
> out an "feature not configured". If you don't like that how about instead
> of changing bb_getopt_ulflags, later on the the applet do:
>
> if(ENABLE_FEATURE_DATE_ISOFMT) {
>   ...
> } else if(isofmt_arg) {
>   bb_invalid_arg('I');
> }
>
> The increaded clarity of the code should be worth the very few
> extra bytes added.

The primary goal of busybox is small size.  I'm trying to come up with the 
clearest way to get that small size, but if I can't come up with something 
that's as efficient as the #ifdefs then we can't move away from the ifdefs.  
(Which would suck.)

The "very few bytes" include at least 4 bytes per extra function argument, 
plus the extra bytes added to the string, plus extra entries in the longargs 
tables...  (Heck, following the above logic we shouldn't bother using the 
existing USAGE() macros in usage.h.)

> If they really want to NITPICK then bb_invalid_arg() 
> could even be #defined to 0.

Alas, we nitpick.

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