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
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
