[Toybox] Entering the home stretch on ifconfig...

[Rob Landley]

So what are the big remaining todo items in ifconfig?

1) Leftover custom option parsing in main()

There are still three if/else chunks in main (lines 633 to 679) that  
aren't in the table (and also aren't indented right). The  
isdigit/default one is sort of halfway between "up" and "netmask",  
should be possible to collate that into the table somehow.

The "hw" one is a large bespoke chunk, easiest thing to do might be to  
move it before the table loop and have it continue, so the error case  
at the end (line 681 now) can just just be what happens when we don't  
match any table entries.

This leaves "add" and "del", which are wrappers around set_ipv6_addr().  
which brings us to the next big chunk of remaining work:

2) ipv6 and ipv4 have duplicate code.

set_address() and set_ipv6_addr(), print_ipv6_addr() is a long tangent  
only called from display_interface().

3) Interface enumeration is tangled.

We put interfaces on a list just to take them immediately off again:  
why not process each entry as we discover it? There are three calls to  
get_device_info(): two from show_iface() and one from readconf(). The  
only thing that calls readconf() is show_iface(), from an else() case  
at the end of the loop. Non-obvious control flow, there.

It looks like show_iface() enumerates /proc/net/dev and then calls  
readconf() to do ioctl() based enumeration. The /proc one gives us RX  
bytes and such, the ioctl gives us a "virtual interface" (ala lo:0 I  
guess?) The only user of the variable indicating the difference is a  
single test in display_ifconfig(). This at _least_ needs some comments.

4) Four calls to xsocket(SOCK_DGRAM); One does AF_INET6 the rest do  
AF_INET.

Those last three seem equivalent? Can we cache the fd and reuse it  
instead of reopening it? Is there a functional difference between the  
AF_INET and the AF_INET6 socket? (No idea where that would be  
documented, have to go read kernel source for that one, I expect...)

5) Still a lot of functions only called from one place.

The functions at the top of the file are:

   get_socket_stream(), get_sockaddr(), address_to_name(),  
set_ipv6_addr(),
   set_address(), add_iface_to_list(), get_device_info(),  
print_ip6_addr(),
   display_ifconfig(), readconf(), show_iface()

These are tangled. get_socket_stream() is only called from  
get_sockaddr(), and that's only called from set_address() and its  
doppelganger set_ipv6_addr().

What does get_socket_stream() _do_? It's a getaddrinfo() wrapper, which  
is not setting AI_NUMERICHOST, so I guess it's doing a DNS lookup? The  
comment on the function says "used to extract the address info from the  
given host ip and update the swl param accordingly." But "ifconfig  
127.0.0.1" doesn't show me lo: so that's not the use case. The swl  
param is a struct sockaddr...

And set_ipv6_address() is checking for a trailing "/int" but the normal  
set_address isn't. Doesn't that set the netmask in ipv4?

   ifconfig lo:1 127.0.0.2/20

Should be equivalent to:

   ifconfig lo:1 127.0.0.2 netmask 255.255.240.0

Time to detour into reading the ifconfig man page...

Rob

P.S. Yeah, traditionally I blog this sort of stuff but I'm trying to  
keep the whole ifconfig series here so I can index it from a  
cleanup.html web page for the site...