Date: Fri, 1 Aug 2025 21:02:24 -0600 From: Alan Somers <asomers@freebsd.org> To: Phil Shafer <phil@juniper.net> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, Damin Rido <rido@freebsd.org> Subject: Re: git: 7b35b4d19630 - main - sockstat: add libxo support Message-ID: <CAOtMX2jZYtY-X8Xs1aT_R2SrkR3QZXCSCUO-y_JWhKS_-j0kcQ@mail.gmail.com> In-Reply-To: <8F928557-328D-46E9-BB53-BDE216693BFC@juniper.net> References: <202507302027.56UKRhb5011315@gitrepo.freebsd.org> <8F928557-328D-46E9-BB53-BDE216693BFC@juniper.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000005160c5063b591b61
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
CC'ing Damin Rido. I haven't had time to look at all of Phil's
observations yet, but I probably will this weekend.
On Fri, Aug 1, 2025 at 11:11=E2=80=AFAM Phil Shafer <phil@juniper.net> wrot=
e:
> 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
> 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.
>
I can't trigger any warnings, with any combination of options. Probably I
don't have the same type of socket that you do. What kind of socket is
triggering warnings for you?
>
> 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 =3D=3D 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) =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
> 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) =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(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
>
--0000000000005160c5063b591b61
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: base64
PGRpdiBkaXI9Imx0ciI+PGRpdj5DQyYjMzk7aW5nIERhbWluIFJpZG8uwqAgSSBoYXZlbiYjMzk7
dCBoYWQgdGltZSB0byBsb29rIGF0IGFsbCBvZiBQaGlsJiMzOTtzIG9ic2VydmF0aW9ucyB5ZXQs
IGJ1dCBJIHByb2JhYmx5IHdpbGwgdGhpcyB3ZWVrZW5kLjwvZGl2PjxkaXY+PGJyPjwvZGl2Pjxk
aXYgY2xhc3M9ImdtYWlsX3F1b3RlIGdtYWlsX3F1b3RlX2NvbnRhaW5lciI+PGRpdiBkaXI9Imx0
ciIgY2xhc3M9ImdtYWlsX2F0dHIiPk9uIEZyaSwgQXVnIDEsIDIwMjUgYXQgMTE6MTHigK9BTSBQ
aGlsIFNoYWZlciAmbHQ7PGEgaHJlZj0ibWFpbHRvOnBoaWxAanVuaXBlci5uZXQiPnBoaWxAanVu
aXBlci5uZXQ8L2E+Jmd0OyB3cm90ZTo8YnI+PC9kaXY+PGJsb2NrcXVvdGUgY2xhc3M9ImdtYWls
X3F1b3RlIiBzdHlsZT0ibWFyZ2luOjBweCAwcHggMHB4IDAuOGV4O2JvcmRlci1sZWZ0OjFweCBz
b2xpZCByZ2IoMjA0LDIwNCwyMDQpO3BhZGRpbmctbGVmdDoxZXgiPlRoaXMgbG9va3MgZ29vZC7C
oCBTb21lIGlzc3VlcyBiZWxvdzo8YnI+DQo8YnI+DQpPbiAzMCBKdWwgMjAyNSwgYXQgMTY6Mjcs
IEFsYW4gU29tZXJzIHdyb3RlOjxicj4NCiZndDsgQEAgLTEzNDUsNjUgKzEzODAsMTA0IEBAIGRp
c3BsYXlfc29jayhzdHJ1Y3Qgc29jayAqcywgc3RydWN0IGNvbF93aWR0aHMgKmN3LCBjaGFyICpi
dWYsIHNpemVfdCBidWZzaXplKTxicj4NCiZndDsgLi4uPGJyPg0KJmd0OyZndDsgK8KgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfSBlbHNlIGlmIChsYWRkci0mZ3Q7YWRkcmVzcy5z
c19sZW4gPT0gMCAmYW1wOyZhbXA7PGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBmYWRkci0mZ3Q7Y29ubiA9PSAwICZhbXA7JmFtcDsgaXNf
dGV4dF9zdHlsZSkgezxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgeG9fZW1pdCgmcXVvdDsgezovJS0qLipzfSZxdW90OywgY3ctJmd0O2xv
Y2FsX2FkZHIsPGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBjdy0mZ3Q7bG9jYWxfYWRkciwgJnF1b3Q7KG5vdCBjb25u
ZWN0ZWQpJnF1b3Q7KTs8YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoH0gZWxzZSBpZiAoaXNfdGV4dF9zdHlsZSkgezxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgeG9fZW1pdCgmcXVvdDsgezovJS0qLipz
fSZxdW90OywgY3ctJmd0O2xvY2FsX2FkZHIsPGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBjdy0mZ3Q7bG9jYWxfYWRk
ciwgJnF1b3Q7Pz8mcXVvdDspOzxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgfTxicj4NCjxicj4NClRoZXNlIGNhbGxzIGFyZSBtaXNzaW5nIGEgZmllbGQgbmFt
ZTsgeW91IHNob3VsZCBoYXZlIHNlZW4gd2FybmluZyBmb3IgdGhpcywgc29tZXRoaW5nIGxpa2U6
PGJyPg0KPGJyPg0KwqAgZm9vOiBtaXNzaW5nIGZpZWxkIG5hbWU6ICUtKi4qczxicj4NCsKgICZs
dDttaXNzaW5nLWZpZWxkLW5hbWUmZ3Q7Pz/CoCDCoCDCoCAmbHQ7L21pc3NpbmctZmllbGQtbmFt
ZSZndDs8YnI+DQo8YnI+DQpZb3UmIzM5O2xsIG5lZWQgc29tZXRoaW5nIGxpa2UgJnF1b3Q7ezps
b2NhbC1hZGRyZXNzLyUqLipzfSZxdW90Oy48YnI+DQo8YnI+DQpQbGVhc2UgYmUgc3VyZSB0byB0
ZXN0IHdpdGggJnF1b3Q7LS1saWJ4bzpXJnF1b3Q7LCB3aGljaCB3aWxsIHJlcG9ydCBtYW55IGlz
c3Vlcy7CoCBBbHNvIHRoZXJlJiMzOTtzICZxdW90O3hvbGludCZxdW90OyBmb3IgY2hlY2tpbmcg
c291cmNlIGNvZGUuPGJyPjwvYmxvY2txdW90ZT48ZGl2Pjxicj48L2Rpdj48ZGl2PkkgY2FuJiMz
OTt0IHRyaWdnZXIgYW55IHdhcm5pbmdzLCB3aXRoIGFueSBjb21iaW5hdGlvbiBvZiBvcHRpb25z
LsKgIFByb2JhYmx5IEkgZG9uJiMzOTt0IGhhdmUgdGhlIHNhbWUgdHlwZSBvZiBzb2NrZXQgdGhh
dCB5b3UgZG8uwqAgV2hhdCBraW5kIG9mIHNvY2tldCBpcyB0cmlnZ2VyaW5nIHdhcm5pbmdzIGZv
ciB5b3U/PC9kaXY+PGRpdj7CoDwvZGl2PjxibG9ja3F1b3RlIGNsYXNzPSJnbWFpbF9xdW90ZSIg
c3R5bGU9Im1hcmdpbjowcHggMHB4IDBweCAwLjhleDtib3JkZXItbGVmdDoxcHggc29saWQgcmdi
KDIwNCwyMDQsMjA0KTtwYWRkaW5nLWxlZnQ6MWV4Ij4NCjxicj4NCkxvbmdlciB0ZXJtLCBJJiMz
OTtkIGxpa2UgdG8gbWFrZSBzb21lIHNvcnQgb2YgY2xhbmcgcGx1Z2luIHRvIHJlcG9ydCBmb3Jt
YXR0aW5nIGlzc3VlcyBhdCBidWlsZCB0aW1lLjxicj4NCjxicj4NCiZndDsgQEAgLTE0MzYsNDcg
KzE1MTAsNTIgQEAgZGlzcGxheV9zb2NrKHN0cnVjdCBzb2NrICpzLCBzdHJ1Y3QgY29sX3dpZHRo
cyAqY3csIGNoYXIgKmJ1Ziwgc2l6ZV90IGJ1ZnNpemUpPGJyPg0KJmd0O8KgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgcy0mZ3Q7cHJvdG8gPT0g
SVBQUk9UT19UQ1ApIHs8YnI+DQomZ3Q7wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBzd2l0Y2ggKHMtJmd0O3Byb3RvKSB7PGJyPg0K
Jmd0O8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgY2FzZSBJUFBST1RPX1NDVFA6PGJyPg0KJmd0OyAtwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBwcmlu
dGYoJnF1b3Q7ICUtKnMmcXVvdDssIGN3LSZndDtjb25uX3N0YXRlLDxicj4NCiZndDsgLcKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgc2N0cF9jb25uX3N0YXRlKHMtJmd0O3N0YXRlKSk7PGJyPg0KJmd0OyAr
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqB4b19lbWl0KCZxdW90OyB7OnBhdGgtc3RhdGUvJS0qc30mcXVvdDssPGJy
Pg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBjdy0mZ3Q7cGF0aF9zdGF0ZSw8YnI+
DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoHNjdHBfcGF0aF9zdGF0ZSg8YnI+DQom
Z3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGZhZGRyLSZndDtzdGF0
ZSkpOzxicj4NCiZndDvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGJyZWFrOzxicj4NCjxicj4NCklzIHRoZSBj
aGFuZ2UgZnJvbSBjb25uX3N0YXRlIHRvIHBhdGhfc3RhdGUgaW50ZW50aW9uYWw/PGJyPg0KPGJy
Pg0KPGJyPg0KJmd0OyArwqAgwqAgwqAgwqBpZiAoeG9fZ2V0X3N0eWxlKE5VTEwpID09IFhPX1NU
WUxFX1RFWFQpIHs8YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGN3ID0gKHN0cnVj
dCBjb2xfd2lkdGhzKSB7PGJyPg0KJmd0OyAuLi48YnI+DQo8YnI+DQpIZXJlIGFuZCBlbHNld2hl
cmU6IHdoZW4geW91IGNoZWNrIGZvciBYT19TVFlMRV9URVhUIGZvciBmb3JtYXR0aW5nLCB5b3Um
IzM5O2xsIGFsc28gY2FyZSBhYm91dCBYT19TVFlMRV9IVE1MIGFuZCBvdGhlciBmdXR1cmUgc3R5
bGVzLsKgIEkgaGF2ZSBhIGZ1bmN0aW9uIGluIGxpYnhvIHRoYXQmIzM5O3MgY3VycmVudGx5IHBy
aXZhdGUvc3RhdGljOjxicj4NCjxicj4NCsKgIMKgIMKgIMKgIC8qPGJyPg0KwqAgwqAgwqAgwqAg
wqAqIEluZGljYXRlIGlmIHRoZSBzdHlsZSBpcyBhbiAmcXVvdDtlbmNvZGluZyZxdW90OyBvbmUg
YXMgb3Bwb3NlZCB0byBhICZxdW90O2Rpc3BsYXkmcXVvdDsgb25lLjxicj4NCsKgIMKgIMKgIMKg
IMKgKi88YnI+DQrCoCDCoCDCoCDCoCBzdGF0aWMgaW50PGJyPg0KwqAgwqAgwqAgwqAgeG9fc3R5
bGVfaXNfZW5jb2RpbmcgKHhvX2hhbmRsZV90ICp4b3ApPGJyPg0KwqAgwqAgwqAgwqAgezxicj4N
CsKgIMKgIMKgIMKgIMKgIMKgIGlmICh4b19zdHlsZSh4b3ApID09IFhPX1NUWUxFX0pTT048YnI+
DQrCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8fCB4b19zdHlsZSh4b3ApID09IFhPX1NUWUxFX1hN
TDxicj4NCsKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHx8IHhvX3N0eWxlKHhvcCkgPT0gWE9fU1RZ
TEVfU0RQQVJBTVM8YnI+DQrCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8fCB4b19zdHlsZSh4b3Ap
ID09IFhPX1NUWUxFX0VOQ09ERVIpPGJyPg0KwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgcmV0dXJu
IFRSVUU7PGJyPg0KwqAgwqAgwqAgwqAgwqAgwqAgcmV0dXJuIEZBTFNFOzxicj4NCsKgIMKgIMKg
IMKgIH08YnI+DQo8YnI+DQpJIGNhbiBtYWtlIHRoYXQgcHVibGljLCBhbGxvd2luZyB5b3UgdG8g
c2F5ICZxdW90O2lmICgheG9fc3R5bGVfaXNfZW5jb2RpbmcoTlVMTCkpIHsgLi4uIH0mcXVvdDss
IGJ1dCBmb3Igbm93LCB5b3UmIzM5O2xsIG5lZWQgdG8gZXhwbGljaXRseSB0ZXN0IGZvciBYT19T
VFlMRV9IVE1MLjxicj4NCjxicj4NCiZndDsgK8KgIMKgIMKgeG9fZXJyb3IoPGJyPg0KJmd0OyAr
JnF1b3Q7dXNhZ2U6IHNvY2tzdGF0IFstLWxpYnhvXSBbLTQ2QUNjZklpTGxucVNzVXV2d10gWy1q
IGppZF0gWy1wIHBvcnRzXVxuJnF1b3Q7PGJyPg0KJmd0OyArJnF1b3Q7wqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgWy1QIHByb3RvY29sc11cbiZxdW90Oyk7PGJyPg0KJmd0OyArwqAgwqAgwqBleGl0
KDEpOzxicj4NCjxicj4NCkkgZG9uJiMzOTt0IGhhdmUgYSByZWFsIGNvbnZlbnRpb24gZm9yIHRo
aXMsIGJ1dCBtYXliZSAmcXVvdDtbLS1saWJ4byAuLi5dJnF1b3Q7IHdvdWxkIGNvbnZleSB0aGF0
IHRoZXJlJiMzOTtzIGNvbnRlbnQgd2l0aCB0aGUgb3B0aW9uPzxicj4NCjxicj4NClRoYW5rcyw8
YnI+DQrCoFBoaWw8YnI+DQo8L2Jsb2NrcXVvdGU+PC9kaXY+PC9kaXY+DQo=
--0000000000005160c5063b591b61--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2jZYtY-X8Xs1aT_R2SrkR3QZXCSCUO-y_JWhKS_-j0kcQ>