[Toybox] [CLEANUP] 898: ifconfig argument parsing, part 1.

Rob Landley rob at landley.net
Sun May 12 19:20:44 PDT 2013


http://landley.net/hg/toybox/rev/898

Design issue for today: replace if/else staircase with loop over a  
table.

The existing option parsing in ifconfig_main() is a giant if/else  
staircase with repeated function calls. It makes multiple calls to  
strcmp() with mostly the same arguments, the first several of which  
call set_flags() (again with mostly the same arguments).

The set_flags() function calls an ioctl to read an interface's flags,  
switch off one set of bits and switch on another set of bits, and then  
call a second ioctl to write those new flags to the interface. The  
existing logic calling set_flags() does a strcmp("string"), and then a  
second strcmp("-string") which mostly does the same thing but with the  
set/clear arguments reversed.

So, new code:

1) Remove redundant curly bracket scope and move local variable  
declarations to the start of function. (Don't reindent everything, just  
the bits we're changing now. Leave the rest indented an extra two  
spaces so the diff doesn't look bigger than it is.

While we're here, use xstrncpy() and xsocket() to simplify setup code.

2) Redo the argument parsing loop to table-based logic.

Declare a structure with three elements: name string, flags to set,  
flags to clear, and fill out an array (named "try") of those structures  
for the 8 names that do nothing but set flags. The set/clear entries  
are in an array so we can easily reverse the order we refer to them in,  
based on a variable that's either 0 or 1. (The first is that variable,  
the second is that variable xor 1.)

Check the argument for an initial - and set a "rev" state variable to  
indicate we're reversing the operation, and increment the argument to  
the next character so we're testing the right string for a match. Next,  
loop through try[] and check each name. If we get a match, call the  
pair of ioctl() calls, setting and clearing bits in between.

What I did here was:

A) replace if/else staircase with a loop over an array.

B) Genericize the - prefix logic to swap set/clear instead of having  
two copies of each entry. (This means that "up" and "down" are not  
perfect synonyms for "-down" and "-up", but they weren't before either.)

C) Inline this use of set_flags(). (We didn't make the function go away  
because there are still more uses of it, but we should be able to suck  
more stuff into this table next time.)


More information about the Toybox mailing list