Re: git: 7b35b4d19630 - main - sockstat: add libxo support

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