This looks good. Some issues below:
On 30 Jul 2025, at 16:27, Alan Somers wrote:
> @@ -1345,65 +1380,104 @@ display_sock(struct sock *s, struct col_widths *cw,
> char *buf, size_t bufsize)
> ...
>> + } else if (laddr->address.ss_len == 0 &&
> + faddr->conn == 0 && is_text_style) {
> + xo_emit(" {:/%-*.*s}", cw->local_addr,
> + cw->local_addr, "(not connected)");
> + } else if (is_text_style) {
> + xo_emit(" {:/%-*.*s}", cw->local_addr,
> + cw->local_addr, "??");
> + }
These calls are missing a field name; you should have seen warning for this,
something like:
foo: missing field name: %-*.*s
<missing-field-name>?? </missing-field-name>
You'll need something like "{:local-address/%*.*s}".
Please be sure to test with "--libxo:W", which will report many issues. Also
there's "xolint" for checking source code.
Longer term, I'd like to make some sort of clang plugin to report formatting
issues at build time.
> @@ -1436,47 +1510,52 @@ display_sock(struct sock *s, struct col_widths *cw,
> char *buf, size_t bufsize)
> s->proto == IPPROTO_TCP) {
> switch (s->proto) {
> case IPPROTO_SCTP:
> - printf(" %-*s",
> cw->conn_state,
> -
> sctp_conn_state(s->state));
> + xo_emit(" {:path-state/%-*s}",
> + cw->path_state,
> + sctp_path_state(
> +
> faddr->state));
> break;
Is the change from conn_state to path_state intentional?
> + if (xo_get_style(NULL) == XO_STYLE_TEXT) {
> + cw = (struct col_widths) {
> ...
Here and elsewhere: when you check for XO_STYLE_TEXT for formatting, you'll
also care about XO_STYLE_HTML and other future styles. I have a function in
libxo that's currently private/static:
/*
* Indicate if the style is an "encoding" one as opposed to a "display"
one.
*/
static int
xo_style_is_encoding (xo_handle_t *xop)
{
if (xo_style(xop) == XO_STYLE_JSON
|| xo_style(xop) == XO_STYLE_XML
|| xo_style(xop) == XO_STYLE_SDPARAMS
|| xo_style(xop) == XO_STYLE_ENCODER)
return TRUE;
return FALSE;
}
I can make that public, allowing you to say "if (!xo_style_is_encoding(NULL)) {
... }", but for now, you'll need to explicitly test for XO_STYLE_HTML.
> + xo_error(
> +"usage: sockstat [--libxo] [-46ACcfIiLlnqSsUuvw] [-j jid] [-p ports]\n"
> +" [-P protocols]\n");
> + exit(1);
I don't have a real convention for this, but maybe "[--libxo ...]" would convey
that there's content with the option?
Thanks,
Phil
