[Toybox] [CLEANUP] ifconfig (commits 878 and 879)

[Rob Landley]

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

The diff adds xsocket(), cleanups up some free() logic (free is defined  
to accept NULL and treat it as a NOP, and we don't need to set a local  
variable on the way out of a function), get_device_info() can return  
void and handle its own error_exit(), remove a half-dozen redundant  
strncpy() of the same value into the same field that's still there, and  
move the llist_traverse() call into show_ifconfig().

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

This is basically inlining three functions in display_ifconfig(), and  
take advantage of the opportunities for further cleanup revealed by  
this.

1) Inline print_hw_addr() at the one callsite (in display_ifconfig()),  
and perform obvious simplifications around it: replace the function's  
variable names to the arguments fed to the function. Move printing the  
"HWaddr" label inside the ARPHRD_ETHER test (since that's the only type  
we support the alternative is potentially printing a label with nothing  
after it), and consolidate the two if statements together. Move the  
declaration of "i" to the start of display_ifconfig() and eliminate the  
other declaration of it later in the function (since it can re-use the  
one variable).

2) get_ip_addr() was only called from print_ip_addr(), and that was  
only called from an if (il->hasaddr) test in display_ifconfig(). So I  
inlined print_ip_addr() into display_ifconfig(), and refactored it so  
the xprintf(" label:value ", one, two) was done within get_ip_addr()  
(which I renamed show_ip_addr). I also defaulted the name field to  
"unspec" and had the tests set it to other values, which made the  
"could be used unsigned" error go away. (That xprintf() is still local  
printing out the varying part, but it's followed by show_ip_addr using  
a hardcoded label "addr", with a space before it, so the result works  
out the same and we avoid repeating the constant string.)

Slight behavior change here: show_ip_addr() always prints a trailing  
space including at the very end. The previous one did a newline without  
a trailing space. I'm ok with it.

I'm still changing xprintf("%10s", " "); to xprintf("%10c", ' '); for  
the trivially smaller argument. This indent is repeated enough it might  
be worth finding a way to factor it out, but making it into its own  
function is kind of silly.

3) get_hw_info() is also only called from display_ifconfig(). First,  
replace it with table-driven logic: the first pass at the table had  
four fields (int type, addrlen; char *name, title;), and it filled out  
hw_info. Then inline it, and notice that only one field from hw_info  
(title) is ever actually consumed, so remove the other two fields from  
the table.

Note that this simplification only showed up when we inlined _both_  
get_hw_info() and print_hw_addr(). In the original print_hw_addr() it  
had if (!hw_info.hw_addrlen) return; but then only had code to print  
out an address for hw_type == ARPHRD_ETHER (which has a nonzero  
addrlen). That test was also the only consumer of addrlen, and only  
cared that it's nonzero, so the constants to set it to various other  
values weren't necessary. Possibly this saves us a header #include or  
#define, I haven't looked yet. In any case, addrlen can be completely  
removed now, we just need type->title associations in the table.

Since that's all struct HW_INFO did, we can eliminate that structure.  
(I forgot to remove the constants for the field lengths, future todo.)  
Instead we find an index into the local table, intentionally stopping  
one before the end so anything unrecognized becomes "UNSPEC". Then just  
print the string straight out of the table rather than making a copy of  
it.

With this checkin, ifconfig is now under 900 lines.

Rob
 1366869614.0