New CONFIG_ guard: IF_FEATURE_BLAH(stuff)

[Rob Landley]

On Thursday 09 February 2006 04:49, Bernhard Fischer wrote:
> On Thu, Feb 09, 2006 at 10:18:29AM +0100, walter harms wrote:
> >hi list,
> >short: i do not like the bb_depend idea. simply add no code for that
> >bit. it would look like this.
> > opt=getopt();
> >#ifdef CONFIG
> > if (opt & CONFIG )
> >   do_something();
> >#endif
> >
> >what it does looks obvious for me.

A small enough instance of anything is generally obvious.  The problem here is 
it doesn't scale.  We have cases with overlapping #ifdefs used repeatedly in 
the same function.  We have functions with variable arguments using more than 
one #ifdef inside a single function call.

> Yes, but the above get's unreadable if you have an applet which has
> several independently selectable arguments. As an example, look at
> pidof.c and expand it to the way you cited above comparing it to how
> pidof.c currently looks in svn.
>
> With the proposed approach, you can just write something akin to
> opt = bb_getopt_ulflags(argc, argv,
>   IF_FEATURE_PIDOF_SINGLE("s") IF_FEATURE_PIDOF_OMIT("o:")
>   IF_FEATURE_PIDOF_OMIT(,&omits));
>
> which is IMO more readable than keeping the old and verbose way to say
> the same.

Slight problem there: the default is an empty set and won't build.  (make 
allbareconfig would find that particular build break.)

The easy fix is to append "" to the first line right after argv, but that 
doesn't result in the smallest possible code.  When there are no options we 
just want to skip the whole call to bb_getopt_ulflags, set opt=0 and let the 
optimizer take it from there.

The default being _no_ options is sort of a special case.

if (ENABLE_FEATURE_PIDOF_SINGLE || EnABLE_FEATURE_PIDOF_OMIT) {
  opt = bb_getopt_ulflags(...);
} else opt = 0;

Yes, this probably saves us less than 20 bytes, but we care about that sort of 
thing. :)

> Furthermore, look at usage.h, where we currently have alot of checks to
> print the proper usage string depending on which features are enabled or
> not. This, too, can be simplified greately when we use the
> IF_BLAH(stuff) Rob is talking about (and which i already prepared
> when adding the platform.h. We will of course drop that prepared
> _usage.h in favour of having the ISSET/ENABLE and IF/DO/younameit in one
> central place iff we agree on it generally and decide on the actual
> names).

I still don't care what the name is, I just want the functionality. :)

> kind regards,
> Bernhard

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