Date: Fri, 1 Aug 2025 13:10:55 -0400 From: Phil Shafer <phil@juniper.net> To: Alan Somers <asomers@freebsd.org> Cc: <src-committers@freebsd.org>, <dev-commits-src-all@freebsd.org>, <dev-commits-src-main@freebsd.org> Subject: Re: git: 7b35b4d19630 - main - sockstat: add libxo support Message-ID: <8F928557-328D-46E9-BB53-BDE216693BFC@juniper.net> In-Reply-To: <202507302027.56UKRhb5011315@gitrepo.freebsd.org> References: <202507302027.56UKRhb5011315@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 =3D=3D 0 &&
> + faddr->conn =3D=3D 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 th=
is, 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 formatt=
ing issues at build time.
> @@ -1436,47 +1510,52 @@ display_sock(struct sock *s, struct col_widths =
*cw, char *buf, size_t bufsize)
> s->proto =3D=3D IPPROTO_TCP) {
> switch (s->proto) {
> case IPPROTO_SCTP:
> - printf(" %-*s", cw->con=
n_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) =3D=3D XO_STYLE_TEXT) {
> + cw =3D (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 funct=
ion 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) =3D=3D XO_STYLE_JSON
|| xo_style(xop) =3D=3D XO_STYLE_XML
|| xo_style(xop) =3D=3D XO_STYLE_SDPARAMS
|| xo_style(xop) =3D=3D XO_STYLE_ENCODER)
return TRUE;
return FALSE;
}
I can make that public, allowing you to say "if (!xo_style_is_encoding(NU=
LL)) { ... }", but for now, you'll need to explicitly test for XO_STYLE_H=
TML.
> + 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8F928557-328D-46E9-BB53-BDE216693BFC>