svn 15195

Rob Landley rob at landley.net
Wed May 31 14:14:56 PDT 2006


Finally getting around to reviewing this:

1) Don't add #if ENABLE to the body of functions.  If you can't honestly 
figure out how to do it cleanly as an if(), there are USE() macros, but with 
just a _little_ effort you can generally get it to be an if().

2) Adding an individual knob for long option support for each one...  Do we 
need that level of granularity?  Is it worth the complexity burden we're 
imposing on people asking them to configure it?  I mentioned a CONFIG_NITPICK 
to hide this kind of trivial question from most people if we started doing 
it, and I think adding these extra config entries _without_ there being such 
a "shut up about it" switch is a bad thing.

3) I think a better thing to do would be to figure out how to move the long 
options to usage.h.  (Since we have to document the applet's arguments there 
anyway, we might as well _define_ them.)  Remember how we were talking about 
a rewrite of bb_getopt_ulflags to not use getopt()?  I have a vague plan for 
having long options look something like:

char **sed_longopts = {
	"x" "fruitbasket",
	"y" "walrus",
	"z" "turquoise",
	""
};

That way we'd have one array of strings and if someone entered --fruitbasket 
that would translate to short option -x.  (We already have that limitation 
now, that all long options must have a short option.  Might as well take 
advantage of it.)  The middle two options are either redundant or used, and 
asking applets to #include getopt for this when they're using 
bb_getopt_ulflags is bad.  In the short term, bb_getopt_ulflags could use a 
data structure like the above to fill out the struct options one when 
parsing.

The advantage of the above is there's _no_ #ifdefs in the code.  
run_applet_by_name() can load the longopts the same way it handles help 
strings for the applet, and it's all transparent, and we're _not_ spraying 
fresh #ifdefs all over the place.

Rob
-- 
Never bet against the cheap plastic solution.


More information about the busybox mailing list