[Toybox] Explaining more ifconfig cleanup.
Rob Landley
rob at landley.net
Tue Apr 9 00:33:56 PDT 2013
Commit 844 continues the ifconfig cleanup:
http://landley.net/hg/toybox/rev/844
Yes, I'm intentionally checking it in in small chunks so it's
understandable what I did rather than just a big magic rewrite.
The first hunk is just cosmetic, wordwrapping things to fit in 80 chars
and twiddling blank lines to be more consistent with the other files. I
added a blank lin to separate the standard toybox #includes (of toys.h
and toynet.h) from the local #includes of system headers.
The file toys.h includes just about all the standard headers that
commands use. Having that be consistent gives ccache more to work with,
and repeating dozens of lines all over the place just to get stuff
that's in libc is silly: there should be an #include <libc.h> in C99, I
don't see the point of making this granular when the linker isn't. I
could just about see having separate #include <libm.h> and #include
<libdl.h> if you have to say -lm and -ldl on the command line, but even
then I'm not sure there's much point to it.
However, the network stuff isn't standardized by posix, and varies a
bit between linux/bsd/macosx or between glibc/uclibc/bionic/musl. This
is why toynet.h is a separate file from toys.h. I need to figure out
what can reasonably portably be added to toynet.h, which means setting
up test environments. Having the headers be local for right now is ok.
(I usually have the #include <linux/*.h> headers be local to the file
using them for similar reasons, there's a lot of "that won't work in
2.6.16" in there. Then again now that dirtree is using the openat()
stuff I'm not sure how old a kernel we care about. Posix-2008 only goes
back 5 years anyway...)
So most likely these #includes will go into toynet.h. Except stddef.h
is already in toys.h, so just delete that.
More pointless #defines: PROC_NET_DEF is "/proc/net/dev". There is no
point in _not_ using the string constant. (It can't even have an
optimization purpose because the preprocessor will substitute in the
string constant.)
Identify several more functions that are not currently used by
anything, and remove them. (Infrastructure in search of a user: put it
in when you need it and not before.)
The print_hw_ether() function an xprintf() repeats ":%02X" six times,
for each element of an array in sequence. Replace that with a for loop.
Slight bit of cleverness: take advantage of the fact that ! is a
logical operator guaranteed to return 0 or 1 by the c99 standard, and
that a string constant is just a pointer to a character array so if you
add one to it you skip the first character. Hence:
for (i=0; i<6; i++) xprintf(":%02X"+!i, address[i]);
Will skip the : the first time through the loop (when i == 0), and then
print the colon the rest of the time.
The function get_ip_addr() does this:
{
static char *ip_str = NULL;
...
if( (ip_str = inet_ntoa(sin->sin_addr)) != NULL)
return ip_str;
return NULL;
}
The whole of that is equivalent to:
return inet_ntoa(sin->sin_addr);
The rest of the changes to that file are whitespace (removing line
breaks from things that fit in a single line anyway) and collating
multiple local variable declarations of the same time into "type a, b,
c";
The print_ip_addr() changes are all whitespace, although it's
whitespace changes that remove 5 lines and thus fit significantly more
in a given size of window where you can see it all.
Rob
P.S. Is this explanation useful?
More information about the Toybox
mailing list