From nobody Sat Aug 2 05:59:49 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bvBxR371yz63dL5; Sat, 02 Aug 2025 06:00:11 +0000 (UTC) (envelope-from daminrido139@gmail.com) Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bvBxR0VWYz3G2Y; Sat, 02 Aug 2025 06:00:11 +0000 (UTC) (envelope-from daminrido139@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-lj1-x232.google.com with SMTP id 38308e7fff4ca-3322d10e29dso18796771fa.0; Fri, 01 Aug 2025 23:00:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1754114403; x=1754719203; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=+YJqxbSdqkdQXrdJnYn6/TGoYwswKaukAPdRWW24YBc=; b=ndVF6/61Jz8L8KHLDGKrPALp/4C3mQsj325/evyvjDO3Ce+cW/MY5vbOzveGRW1wgP oL3HqmWFAlC+3hrLvU5e+VgA9MmLbzg0/nb60x2MmYGgxFjohTGOvOhPZGu9f2o9cCF1 l2mwY4P0smCeUOh6WQWAJZSfsbsfOTm2k1n564izGFOZ0R++VHWeMQ8+WhVFYda+uydv ZzDZLRm8PjodAKIcchLxuB6xisrOvtKq8ZY6I4B9/ZxzNY6Ek7B5H0P3cb1n9JpQIBMW cpTn7s4MLfKdkGAZL67+sVXgOp6UQ7tE1GksDO+VtPiVXA7IA6yM1Kb9T4plEBjMCnqG jp4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1754114403; x=1754719203; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+YJqxbSdqkdQXrdJnYn6/TGoYwswKaukAPdRWW24YBc=; b=jJvV+z4lK7uWju37oOdd0uM6wGDda01wqusEBWx7LlpcYhaENq/UgLJYvFgfjBr3nf cpZ9jEvbHfjGz2JLI4mI5G35j/Jpd8l4Rdkdux9787Xv51AJPSk573m50Doq3FW4lvP7 IP/lT1iWej6JIUkm4j2d5BjvBDiIj8rpJ60yTejzeYKDpIj3DG3pel+McxFW9q6XZk0G Bjv8dpbTIGbaWhQMNbwQTW5z/wkF/RvRYkjtreGQ3h93rbYHgDjRVz99u2B9SvqUimdO EYhg3q3nMuUSNpN8LoAnTUGLhVwhCniDr1LRgwoHOZ2KNucT9feGmS7vITQw39ySxGVx 85tQ== X-Forwarded-Encrypted: i=1; AJvYcCU7k+pjfnJwLPfIoixP8VEIw4PfTHC3fxdLnjj5MWKexXpMGxBRJXwluRGiMO8IzQka6J+zCQ==@freebsd.org, AJvYcCV3qscCCxx9baSu64pOOGNMJrUcYaRWGzPyvyYGnOgUqXm8UtCtZOUWXI1FMw6BhUkKpt0fEnWEFUsb7pKFUG8XYay8NxA=@freebsd.org, AJvYcCVAUgyLsXp6BZw3BTOYiZfHugIbtu4ZQXuWPdnquO0BWuHUU3Il5Gv7d1TVqeVPNv/yV+8T4WnfG4c8mDTvOYs=@freebsd.org, AJvYcCXqtnPCMgzCNw2Y/8qph7TFxOyXLlNvNmsgNgwGcpckEhhviwKW0ZyCVHQM6JTb+NUus2O0XqQLVXbZlSTaE8rlM+Aa@freebsd.org X-Gm-Message-State: AOJu0YzDQPQOKQ4HC3XcubVvzhJ+hKsSW80saX9Q2pOgG7YKSQdr5W4A 5QvZXSlZ3+5kvEGr/b1vgf+EAraf4mlHEo1cKJjOPC20nO6tCc78W42C4Ozw3KCWQrJeOoqaRFj Nh02XvoqP2yqx1fG7X3GFk0kkhd++hoPOud2rSZChIw== X-Gm-Gg: ASbGncsnhOPgqdXuvL1VL6PazhTQ+H21skTS+Gj2ix6Ajoynr7mCR87uSAfnIMjKdtN FPKaV3Equ90qvlCMeQxGFwPYv6cYNQQ4fgikJCyaQbXhdXcrL1J1zJ5f1PLWDunCdGubaiqnn4B jUihIUFsuFs8nrntSY8UnUDB443djKevf5wwT0HPNdVUvpZOA5jFm5QVi9V73i5wvj0C2cdE6Kc kf+AiuGqucdYXunNyUIheR0mrsChzM= X-Google-Smtp-Source: AGHT+IGVLZBkpgN4XyB8cNi+7qcFhNK3jQVfUnP6KYe8OXJwQvsS2fMlwut3fcwNpqoepqd2nRBWjVosWf8P1EFMldQ= X-Received: by 2002:a05:6512:308e:b0:55a:e1f6:bd98 with SMTP id 2adb3069b0e04-55b97abd6dcmr567973e87.6.1754114402703; Fri, 01 Aug 2025 23:00:02 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 References: <202507302027.56UKRhb5011315@gitrepo.freebsd.org> <8F928557-328D-46E9-BB53-BDE216693BFC@juniper.net> In-Reply-To: From: Damin Rido Date: Sat, 2 Aug 2025 11:29:49 +0530 X-Gm-Features: Ac12FXweD79cqmQTpT-ynD8Quw5F80WVh1J0fBKqjbf_cDHeFjkBVgEXjg1B3fQ Message-ID: Subject: Re: git: 7b35b4d19630 - main - sockstat: add libxo support To: Alan Somers Cc: Phil Shafer , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, Damin Rido Content-Type: multipart/alternative; boundary="000000000000de5d04063b5b9587" X-Rspamd-Queue-Id: 4bvBxR0VWYz3G2Y X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] --000000000000de5d04063b5b9587 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thank you for your valuable feedback. > @@ -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 >> ?? >> >> 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 deliberately omitted field names in some xo_emit calls when the output style was XO_STYLE_TEXT, since I assumed field names wouldn't matter for human-readable formats. I also couldn=E2=80=99t trigger any warnings with --libxo:W. However, to stay consistent with xolint expectations and ensure correctness across all styles, I=E2=80=99ll add proper field names in my ne= xt commit. > >> > @@ -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_stat= e( >> > + >> faddr->state)); >> > break; >> >> Is the change from conn_state to path_state intentional? >> > The change from conn-state to path-state was a mistake. Thanks for catching that. I=E2=80=99ll revert it in my next commit. > >> > + 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. >> >> I initially assumed that only TEXT is intended for human-readable output, and that XML, JSON, and HTML are all machine-readable formats. That=E2=80= =99s why I treated TEXT separately in my formatting logic. Also, I use space-padding specifically for XO_STYLE_TEXT to produce a neat, structured, table-like output, which I assumed wouldn=E2=80=99t be necessary for HTML or other for= mats. Could you please clarify why HTML should be handled similarly to TEXT in this case? > > + 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? >> >> Sure, that makes sense. I=E2=80=99ll update it to use "[--libxo ...]" to in= dicate that there are options. Thanks for the suggestion! --000000000000de5d04063b5b9587 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thank you for your valuable feedback.

=
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex"> > @@ -1345,65 +1380,104 @@ display_sock(struct sock *s, struct col_width= s *cw, char *buf, size_t bufsize)
> ...
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0} else if (laddr->address.ss_len =3D=3D 0 &&=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0faddr->conn =3D=3D 0 &&= ; is_text_style) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0xo_emit(" {:/%-*.*s}", = cw->local_addr,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cw-&g= t;local_addr, "(not connected)");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0} else if (is_text_style) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0xo_emit(" {:/%-*.*s}", = cw->local_addr,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cw-&g= t;local_addr, "??");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0}

These calls are missing a field name; you should have seen warning for this= , something like:

=C2=A0 foo: missing field name: %-*.*s
=C2=A0 <missing-field-name>??=C2=A0 =C2=A0 =C2=A0 </missing-field-= name>

You'll need something like "{:local-address/%*.*s}".

Please be sure to test with "--libxo:W", which will report many i= ssues.=C2=A0 Also there's "xolint" for checking source code.<= br>

I deliberately= omitted field names in some xo_emit calls when the output style was XO_STY= LE_TEXT, since I assumed field names wouldn't matter for human-readable= formats. I also couldn=E2=80=99t trigger any warnings with --libxo:W. Howe= ver, to stay consistent with xolint expectations and ensure correctness acr= oss all styles, I=E2=80=99ll add proper field names in my next commit.
=C2=A0

> @@ -1436,47 +1510,52 @@ display_sock(struct sock *s, struct col_widths= *cw, char *buf, size_t bufsize)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s->proto = =3D=3D IPPROTO_TCP) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0switch (s->proto) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0case IPPROTO_SCTP:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0printf(" %-*s", cw->conn_state,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sctp_conn_state(s->state));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0xo_emit(" {:path-state/%-*s}",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cw->path_state,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sctp_path_state(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0faddr->state));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;

Is the change from conn_state to path_state intentional?

The change from conn-state to pa= th-state was a mistake. Thanks for catching that. I=E2=80=99ll revert it in= my next commit.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (xo_get_style(NULL) =3D=3D XO_STYLE_TEX= T) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cw =3D (struct= col_widths) {
> ...

Here and elsewhere: when you check for XO_STYLE_TEXT for formatting, you= 9;ll also care about XO_STYLE_HTML and other future styles.=C2=A0 I have a = function in libxo that's currently private/static:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* Indicate if the style is an "encod= ing" one as opposed to a "display" one.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0 static int
=C2=A0 =C2=A0 =C2=A0 =C2=A0 xo_style_is_encoding (xo_handle_t *xop)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (xo_style(xop) =3D=3D XO_STYLE= _JSON
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 || xo_style(xop) = =3D=3D XO_STYLE_XML
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 || xo_style(xop) = =3D=3D XO_STYLE_SDPARAMS
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 || xo_style(xop) = =3D=3D XO_STYLE_ENCODER)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return TRUE;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return FALSE;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

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.


I initially a= ssumed that only TEXT is intended for human-readable output, and that XML, = JSON, and HTML are all machine-readable formats. That=E2=80=99s why I treat= ed TEXT separately in my formatting logic. Also, I use space-padding specif= ically for XO_STYLE_TEXT to produce a neat, structured, table-like output, = which I assumed wouldn=E2=80=99t be necessary for HTML or other formats. Co= uld you please clarify why HTML should be handled similarly to TEXT in this= case?
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0xo_error(
> +"usage: sockstat [--libxo] [-46ACcfIiLlnqSsUuvw] [-j jid] [-p po= rts]\n"
> +"=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [-P pro= tocols]\n");
> +=C2=A0 =C2=A0 =C2=A0exit(1);

I don't have a real convention for this, but maybe "[--libxo .= ..]" would convey that there's content with the option?


Sure, that makes sense. I=E2=80= =99ll update it to use "[--libxo ...]" to indicate that there=C2= =A0are options. Thanks for the suggestion!
--000000000000de5d04063b5b9587--