http://landley.net/hg/toybox/rev/906
I started by doing the table cleanup I mentioned last message, bringing the table back down to three members. I took out the array and made all three members separate, called "name", "on", and "off", and just had the flag setting logic doing two ? : swaps to handle rev. what used to be the t->addr != 0 case is now ((t->off | 0xff) == 0x89ff), in which case I zero off before continuing. (I copy on and off to local variables so I don't muck up the table in case people do more than one instance of the same command, although why they'd do that... Oh well, it also makes the ? : stanzas fit in 80 chars more easily.)
Another cleanup: remove every instance of char *req_name in function arguments. They stopped being used when xioctl() went in, but they're still vestigially there in a lot of places.
The rest of this one is basically just a frenzy of inlining: set_metric(), set_qlen(), set_memstart(), and set_ioaddr() all do basically the same thing: read the interface state with SIOCGIFMAP into ifre, strtoul() a command line argument and assign it to a field of ifre, and xioctl() it back to the interface. In fact set_irq() is doing pretty much the same thing, but it's doing unnecessary casting and pointer arithmetic to refer to "ifre.ifr_map.irq". I don't know why. All of it's called exactly once, so having it be a function serves no obvious purpose.
I also don't know why the ioctl number for the second call to xioctl() isn't in each function, but is instead passed in as an argument from the caller. Presumably this is intended as some kind of documentation of what the function we're calling is actually doing... except over half of them are repeats of SIOCSIFMAP? (Get interface hardware mapping, set interface hardware mapping. And the get is hardwared, it's just the set that isn't...)
Yes, this is fairly regular code that can probably get sucked into the table driven logic in a future cleanup round. Especially since most of them (possibly all) are actually union members buried inside the structure, so we wouldn't even have to store separate offsets in the table.
I have been wondering if the "get" and "set" should be moved outside the loop. That way we don't do redundant gets and sets, but just do one get, adjust everything, do one set. This might help with the "default" case at the end of the remaining if/else staircase, which is the only remaining user of set_flags() and is preventing the (already inlined) function from going away. I'm not quite sure how to put the isdigit() case into the table yet, corresponding to "ifconfig eth0 192.168.1.1". Eh, worry about it in a future pass.
At the very end, slightly better error reporting. When error_exit() reports a bad argument, say what the argument was.
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
