[Toybox] Ifconfig cleanup: bonus round!

[Rob Landley]

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
 1385264489.0