[Toybox] vconfig cleanup pass 2.

[Rob Landley]

Once I collate and rsync this should become  
http://landley.net/hg/toybox/rev/1106

Second pass on vconfig cleanup:

Looking further at strtorange(), thinking of genericizing it and moving  
it into lib: Despite the original name starting with strtol, this  
returns an int and accepts int limits? Didn't notice that last pass.  
Long is better.

The lib/args.c logic for converting a string to an int in a range has  
an error message "-%c < %ld", which gives the name of the argument that  
violated a constraint and doesn't have any english in it at all (no  
need to translate). It's also setting a default value, which this  
isn't. This version is not a clean drop-in replacement for the  
lib/args.c range logic. Best leave them separate for now. Still might  
move strtorange() into lib/lib.c, but not this pass.

Whitespace and brackets: more removal of BSD code style brackets, and  
inserting a space in flow control statements that look like functions,  
ala "if (blah)". Also switch strcmp() == 0 to !strcmp().

Inline MAX_VLAN_ID: #defining a value and then using it exactly once is  
not
a net gain.

Replace socket() with xsocket(). No need to comment that we're using  
socket. (Comments should explain something the code itself doesn't.) No  
need to set a default value when fd is initialized if we're just going  
to unconditionally overwrite it here.

The set_name_type bit is operating on an enum, so it can be turned into  
a for loop checking an array of strings and the value is just the  
position in the array. And the error message might as well print out  
the types we accept because they're not currently documented anywhere  
else. (I'm tempted to fluff out the usage message to list these four  
types and explain what they _do_... but I dunno what they do. Can't  
document what I don't know.)

Why is name_type given a default value (VLAN_NAME_TYPE_PLUS_VID)? We  
require at least two arguments, therefore this can't _not_ be set, and  
if it's not one of the known values we error_exit. (Actually  
perror_exit() which prints "No error" when errno is 0, which it should  
always be here: error_exit() without the p doesn't do that.) Anyway,  
deleted name_type since it's no longer used...

The "Store interface name" bit can be replaced with xstrncpy(). (Again  
perror_exit() appends the errno string, but we haven't called a  
function that set it to anything, use error_exit() when you don't need  
to report errno.) While I'm here I should make xstrncpy report the  
string and the length limitation. No need for an intermediate  
interface_name variable, adjust the message at the end to just use  
toys.optargs[1];

The device1 buffer is char[24], but we're limiting it here to 15 (or 16  
with the null terminator). Why?

The "store interface name" bit doesn't need to be an else: the  
set_name_type stuff exits on success or failure so we don't get there  
otherwise.

I'm not familiar with the vconfig command, is it normally this chatty?  
xprintf("Successful set_name_type for VLAN subsystem\n"); Ordinarily a  
unix command is silent on success and only outputs stuff on failure or  
if somebody requested information... I'd _like_ to turn that into  
xioctl(); return; but I dunno how closely we're mirroring compatability  
with some other implementation. (I'll probably do that cleanup  
anyway...)

We don't need all these "else request.u.blah = 0" since we memset it  
all to 0 at the start of the function.

Move the WARNING for add 1 into the add block so we're not doing the  
strcmp("add") twice.

I have a largeish problem at this point: I don't have a vconfig test  
suite. I've reached another good stopping point and the command  
compiles again, but I made changes that weren't just whitespace and  
can't really test it. I dunno what success looks like with this  
command, or how to test all the fiddly corner cases. (In theory I can  
set something up with a couple qemu instances and tun/tap, but I dunno  
how...)

Oh well, check it in anyway...

Rob
 1383423716.0