[Toybox] netstat cleanup.

Rob Landley rob at landley.net
Sat Jun 11 20:12:47 PDT 2016


Yeah, yeah I should blog this, but the lists's here. :)

Elliott's patch to netstat did a lot of:

  -  if (!strcmp(label, "tcp")) {
  +  if (!strncmp(label, "tcp", 3)) {

Which made me raise an eyebrow for a couple reasons: 1) we have
strstart() in lib/ that avoids having to manually supply the length of a
string that you just supplied. 2) The reason for the change is we only
match the _start_ and not the whole thing.

The reason for that is this change:

       show_ipv4("/proc/net/tcp",  "tcp");
-      show_ipv6("/proc/net/tcp6", "tcp");
+      show_ipv6("/proc/net/tcp6", "tcp6");

Which immediately makes me go strrchr(argv1, '/')+1 gives you the second
string, there's no reason to duplicate that string, what's wrong with
this code... and I started digging in andfound that show_ipv4/show_ipv6
are _mostly_ the same code, and then call common functions further down
anyway (which do the strcmp above to distinguish whether they're doing
tcp, udp, or raw versions of stuff.)

So I pulled out the blowtorch and started digging...

Skipping what I did yesterday, today I've been poking at
show_unix_sockets(), which sscanf's values of out /proc/net/unix. It's a
little non-obvious that's what it's doing because there's exactly one
caller of show_unix_sockets() that feeds in the filename as an argument
to disguise the fact that this function is hardwired to operate on a
single source of input. (Making that stop is a todo item.)

There are some string arrays at the top (they didn't used to be there
but I collated them), and the ss_type[] array is bounds checked using
macros which grep found in bits/socket_type.h, which has a SOCK_PACKET
type I might as well add.

I changed the index types from signed to unsigned, because even the C
over-optimizing loonies haven't declared _unsigned_ wraparound mystic
black magic knowledge that nobody can POSSIBLY understand how ones
complement works and thus the compiler is free to make stuff up when it
sees that. And that means I can take the 1-based indexes ofthe macros
and subtract 1, and 0 becomes (2<<32)-1 and is caught by the
>=ARRAY_LEN(blah) check and squashed to "UNKNOWN". I have to test
SOCK_PACKET specially and adjust its value by hand because for some
reason the kernel guys made that nonlinear, but oh well.

But ss_state[] is _not_ using macros I can easily find in a header, so
let's dig into the linux source to see what it's exporting!

Grep under fs/proc for '"unix"' doesn't hit anything, so let's see if
it's under net/ instead... yes, net/unix/af_unix.c function
unix_seq_show(). The seq_printf() has a hardcoded 0 in field 3, so the
netstat.c code reading "label" is always going to get a hardcoded 0 so
let's eliminate the variable, * out another field in the sscanf()  so it
doesn't record the value and then hardwire "unix" into the output string.

Ok, on to state... It's:

  s->sk_socket ?
    (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTED : SS_UNCONNECTED) :
    (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING),

And those macros are in include/uapi/linux/net.h which has 5
values, 0 being SS_FREE. The above can't RETURN SS_FREE, does anything
anywhere in the kernel ever actually use SS_FREE? Let's see, the vmware
vsockets driver uses it twice, nfs's sunrpc layer puts it in a table,
and net/caif/caif_socket.c has an assert saying this should never
happen. So no, it's not used by any sane code.

Right, since the unused SS_FREE was the 0 value anyway, delete the
"FREE" out of the table, shift everything down by one again, and now the
code for state looks the same as the code for type.

While we're at it, there's no point having a temporary char * to copy
this into, just index the array in the printf()...

Anyway, grinding away. As you do...

Rob


More information about the Toybox mailing list