On 04/13/2013 07:56:34 PM, Isaac Dunham wrote:
Hello,
I took a look at ifconfig to see about show_help vs toys.exithelp,
and I saw this repeated 16 times ("string" is the only thing that
changes):
} else if (!strcmp(*argv, "string")) {
if(*++argv == NULL) {
errno = EINVAL;
show_help();
}
set_string(...)
This looks like a pretty obvious candidate for a helper function that
does
more than show help and exit.
Indeed.
It would seem one could do something like this:
void nullarg_help(char * args)
{
if (*args == NULL) {
errno = EINVAL;
toys.exithelp++;
error_exit("missing argument");
}
}
..
} else if (!strcmp(*argv, "string")) {
nullarg_help(*++argv);
set_string(...)
I was actually thinking something table driven, trying to incorporate
the actual work the option did into the table (possibly with a function
pointer as one of the table entries), but hadn't gone through and
triaged all the cases yet.
I probably got the pointers wrong, but that should show the general
idea.
It will drop 40 lines (16 occurrances * 3 lines saved - 8 lines to
define).
Any thoughts on this?
You've definitely got the right idea, I was just thinking of taking it
further. I'll try to code something up in the morning to show what I
had in mind.
(On the other hand, the changes you're suggesting wouldn't stop me from
doing the changes I'm thinking of on top of them. If you'd inclued a
patch I'd probably apply it now.)
In general, I try to do the simple cleanups first to get the code
compact enough I can wrap my head around it and do design changes once
I've run out of landscaping work. At some point I need to read the
entire file start to finish and understand what it's _doing_. I haven't
sat down and done that yet. When I try, I keep finding... things.
Rob
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net
