s/then/them/

Maybe s/dump/print/ as well

Picking some random lines; I think the header ordering was right before? style(9):

Kernel include files (sys/*.h) come first.
... The
remaining kernel headers should be sorted alphabetically.
...

Leave a blank line before the next group, the /usr/include files, which
should be sorted alphabetically by name.

#include <stdio.h>

I'm guessing that you made this change because sys/types.h cowardly refuses to define the same types in userspace as it does in the kernel, and thus you need at least stdbool and probably stdint, if not others, before the sys/qmath or sys/stats or sys/arb includes?

I might fix that by just having those sys/ headers recursively include their dependencies in userspace builds only :-/. Or probably more contentiously, sys/types.h could be modified to include the userspace headers for the types it normally defines for the kernel.

You could also just live with a mostly style-correct and unchanged ordering, but add the handful of needed userspace headers above the sys/ ones (probably not all of these are needed).

Doesn't this include need to be conditional on WITH_STATS? Without it, the file may not be installed?

FreeBSD_print_measurements()?

(Is there a real active upstream for this program? Edit: I guess we regularly import updates from OpenBSD, ok.)

There's another close() case here, although I agree it doesn't make sense to print stats in this instance (unexpected poll error). It doesn't make sense to close() immediately before err() exit, either, but I guess we inherited it from OpenBSD.

Shouldn't we close and print measurements in this case?

Maybe this is just me, but sb is commonly used for struct sbuf pointers, so the name here makes reading the following code a little confusing to me. You don't have to make any change if you don't want to, but I'd probably use a distinct name for the statsblob, like stats or statsb.

Don't cast malloc in C

This kind of retry classically has the problem of never catching up to a growing structure. Although I guess if the socket is mostly shutdown maybe we can reasonably expect the true size to be static. Nevermind! :)

I think you intended to initialize error rather than errno; the former makes the logic more correct (it's not now) and the latter is already set by realloc() on failure.

But I'd suggest just doing err(1, "realloc"); here instead; discussed below.

This is not a great err message; it probably makes sense to at least specify TCP and stats. And it's totally misleading if realloc failed.

This function doesn't set errno on failure, it just returns a value. So you want errc(1, error, "stats_blob_tostr");

Is the same proto:key really overloaded for both enabling statistics gathering and dumping the output blob? Can programs actually tell if TCP_STATS is enabled or does getsockopt(TCP, TCP_STATS) try to dump a stats blob into an int?

I think I'd prefer the third one, seems least intrusive, even if somewhat ugly.

The header will be installed, it comes from sys/sys/, not from lib/libstats/.

Hm, you're right.

Not sure what's the best way to fix it. What do you think about this version?

I'll investigate.

It seems you can't call getsockopt(3) on a connection that has been shut down; in this particular case it trips this check in sys/netinet/tcp_usrreq.c:tcp_ctloutput():

if (inp->inp_flags & (INP_TIMEWAIT | INP_DROPPED)) {
        INP_WUNLOCK(inp);
        return (ECONNRESET);
}

Huh. That seems like a kernel bug to me, although obviously extremely tangential to this review. No objection to this for now.

This can give the wrong error message sometimes:

nc: getsockopt(TCP_STATS) failed; kernel built without "options STATS"?
nc: getsockopt: Connection reset by peer

We should not emit the "kernel built without STATS" in this common case, it is misleading