[Toybox] [take 2] Ifconfig cleanup: bonus round!

[Rob Landley]

Now with (hopefully) less broken wordwrapping. (Thanks balsa! You're a  
horrid email client.)

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

My ifconfig cleanups stalled a while back when I got derailed doing  
ipv6 research, an area I'm not that familiar with because I don't  
personally use it. But the musl-libc has some ipv6 address tests that  
got me looking at this area again, so:

display_ifconfig():

The fopen("/proc/net/if_net6") has wrong whitespace (while() with  
function spacing instead of flow control statement spacing, and no  
blank line between the variable declarations and non-declaration code),  
which I leave as a hint to myself that I need to look at this area more  
deeply.

No need to zero iface_name[] during its declaration if the sscanf() is  
going to initialize it. Similarly, nitems is overwritten a couple lines  
after initialization.

The fgets() is reading a structure parsed by sscanf() which has to have  
32+8+2+2+2+15 plus 6 whitespace characters for a grand total well under  
128. So using the whole of toybuf is probably overkill...

Don't need "sizeof(ipv6_addr) / (sizeof ipv6_addr[0])" because  
ipv6_addr is a char and we know how big a char is.

Unwind the if (nitems = 4) block because it's 4 lines and two separate  
if statements can be two lines. (Shorter code that isn't significantly  
more complex is generally a win.) Similarly, collate the two int  
variable declarations onto the same line, which is still nowhere near  
80 characters... except we've already got an "int i" in the enclosing  
scope, so that declaration is redundant.

char *p; has too large a scope to grep for where it's used, so rename  
to pp. Then reuse it in the fopen() to copy the path and use the  
pathname in the perror_exit().

Redo the ipv6_addr read to be the start of the string and count down  
instead of up inserting the colons. That way we can do &3 instead of  
%5, and as a bonus it copies the existing string's null terminator.  
(The if () if () is because even though I _think_ && is a sequence  
point so (i++ && i) is deterministic, I'm not _sure_. I know if (i++)  
if (i) is.)

I'm still a little iffy about the inet_pton()/inet_ntop() pair there.  
We're parsing the a set of 32 hex digits itno a colon separated string,  
converting it to a binary address, and then converting that _back_ into  
ipv6. I'm aware that this squishes runs of zeroes and does the ::  
thing, but as long as we're parsing anyway it might be better for us to  
do that ourselves.

Now the potential reason to use inet_ntop() is that squishing the first  
run of zeroes into :: is trivial but squishing the LONGEST run of  
zeroes isn't. (Or at least requires two passes.)

Let's see, how do zero runs squish in practice...

   $ sudo ifconfig eth0 add 1:2:3:4:5:6:7:8:9:ff
   1:2:3:4:5:6:7:8:9:ff: Resolver Error 0 (no error)

Oh hey, that's nice. (Ubuntu 12.04 does that. People haven't really  
debugged ipv6 much, have they?)

   $ ifconfig eth0 add 0:2:3:4:5:6:7:ff
   $ ifconfig eth0 add 0:0:3:4:5:6:7:ff
   $ ifconfig eth0 add 1:0:3:4:5:6:7:ff
   $ ifconfig eth0 add 1111:2222:3333:4444:555:66:7:ff
   $ ifconfig eth0
   eth0      Link encap:Ethernet  HWaddr dc:0e:a1:52:92:77
             inet6 addr: 1111:2222:3333:4444:555:66:7:ff/128  
Scope:Global
             inet6 addr: 1:0:3:4:5:6:7:ff/128 Scope:Global
             inet6 addr: ::3:4:5:6:7:ff/128 Scope:Global
             inet6 addr: 0:2:3:4:5:6:7:ff/128 Scope:Global

Um, the toybox one isn't printing any inet6 addresses? Ah, because it's  
looking for /proc/net/if_net6 and the file is if_inet6 with an i. (My  
bad, sorry. I broke it in an earlier cleanup round and had no ipv6  
tests ready. Trying to rectify that now...)

Anyway, the zero eliding behavior is complicated enough that relying on  
libc to do it for us is probably a good idea, so let's make it work:

The next reason we get no ipv6 output is display_ifconfig() is called  
from show_iface() with an argument that lives in toybuf, and then we  
overwrite toybuf so the strcmp(name, iface_name) never triggers. Easy  
fix: change the display_ifconfig() call from show_iface() to use the  
variable that lives in argv[] instead of the one that lies in toybuf.

   if (inet_pton(AF_INET6, ipv6_addr, (void *)&sock_in6.sin6_addr) > 0)  
{
     sock_in6.sin6_family = AF_INET6;
     if (inet_ntop(AF_INET6, &sock_in6.sin6_addr, toybuf,  
sizeof(toybuf))) {

Riiiight. so we parse it as AF_INET6, we set AF_INET6 in the struct,  
and then we feed AF_INET6 to the expansion function. Do we really need  
all three? (Tries commenting out the middle one... still seems to work.)

Don't need an int j; if int i isn't used in scope here...

Ok, that's probably enough for one pass.

Rob
 1385792907.0