From nobody Mon Apr 28 12:23:27 2025 X-Original-To: dev-commits-src-main@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 4ZmN0031xdz5vJ9D; Mon, 28 Apr 2025 12:23:28 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4ZmMzz6kDZz3g52; Mon, 28 Apr 2025 12:23:27 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745843007; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Fp5IjJ+/WDoU8wqEIDq2yjCT6GTCL3NBR7U8WzoITEw=; b=PiEsw6eZ3aLzOZ+FkkYOIhLEGPPdaDeDABdoJRhlSCKF/pnhidwSUk2YH8EI2F9yt3xhO4 TWvEtrUN+wS8azbbKYV1/a6rHJD3ggCTf3yi1TCFMnbYwygY4ygF7Md9ZRyn5AMEOxNras scXFmR3pE1xGFdRQV5Fvf+CmUJ4iT6ICjM8VJ/Rbd6SdRKwToMVHDRkjX2+wvraNHLGWe4 43s9MLsQpdM4ygrKH7W7Oznzusr+sWCZKIjFP6fZFpf/0aF27qhC/p6njY8pLKksdA5lxd MB5gmAgU0pC3r7Z5m2ZtBa/rpP6p5+kknxWie6Mac59o7PtWKC8YtOceQFeDQg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1745843007; a=rsa-sha256; cv=none; b=kl0PrKtSkwn04Q1+vAJOmQ7i2mGo+XAdsSjm1XnnFGl/NlbNLGVtvB6uaTCzmozM+LkLY0 uS3HNqjeo97hoT32+HZ9dmRwR/lrO2XEkCbB4o/O/2pBg6syi3/pDvU6BGNj/qcdVRl6KF jqUYI8wTWEdbaudHkYeDJKNAM0KiJRAOb1kC268UOpq1MPNocVviRxk5QJmgan82xQ0p4u EOGzphS/k/EH2V3xaQv/DsdxEO67iW7d4P7cOmxCFpOCupUTYZS6p0aZ0RLZeuq603Nltf bYLahIMDsT00c9pu1ZbsrBqpfQGQeoQemuVPSZHK8UvPOMUMfVWBcWa1DI6vSQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745843007; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Fp5IjJ+/WDoU8wqEIDq2yjCT6GTCL3NBR7U8WzoITEw=; b=hzomVo6HbkOwncSp+GU3LdSXYbluRVLgXNpfSiJO8n4RyfbIyPcWxgAUqZpIA6Wn1uAJpg 7uS48zATICFqiengz3JqKhsHBFDsvHE4XlNWf124FsN9XFVVBepSVEt0ipYwXX+xJzFeDl teuB6D0LBFFISolcNPGvLc3kQz4viRGyZq7pWoUIywd9qTLWokvOenXPRIt5iuOOXRyKW+ dGn8YDYY2+1xeKVEL2Osnvafs4QlmOjf59/6QkWAFbQeBr+8iBip/KE+DF0HqFlgw0NYTr Dt8auFaT+9ZUepj6g1ikqZkGNdMSFJVy/dqQE9ARfuJErD5+kpzc7v64I+a0/Q== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4ZmMzz4g49zbW2; Mon, 28 Apr 2025 12:23:27 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 53SCNRbv025363; Mon, 28 Apr 2025 12:23:27 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 53SCNR8d025360; Mon, 28 Apr 2025 12:23:27 GMT (envelope-from git) Date: Mon, 28 Apr 2025 12:23:27 GMT Message-Id: <202504281223.53SCNR8d025360@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 193e68d5174f - main - ps(1): Aliases: Resolve once, merge specifications List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: olce X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 193e68d5174ffde70acc427f9d830797df9a13cb Auto-Submitted: auto-generated The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=193e68d5174ffde70acc427f9d830797df9a13cb commit 193e68d5174ffde70acc427f9d830797df9a13cb Author: Olivier Certner AuthorDate: 2025-04-25 13:20:08 +0000 Commit: Olivier Certner CommitDate: 2025-04-28 11:57:01 +0000 ps(1): Aliases: Resolve once, merge specifications With this change, an alias keyword is resolved once and for all by merging specifications of the keyword (or chain of keywords) it points to. This merge in particular determines the final attributes of the alias keyword, as well as the final keyword it points to, which uniquely determines which information is printed (all non-alias keywords are assumed to display different data). Also, the alias resolving code has been moved into the resolve_alias() function and helpers (e.g., merge_alias()). Aliases are still resolved lazily as needed by default. The new top-level resolve_aliases() function is used to resolve them at once at program startup if ps(1) has been compiled with PS_CHECK_KEYWORDS defined. Else, it can also be called directly from a debugger. This is in preparation for removing columns that actually display the same information in a subsequent commit, as this requires being able to (quickly) determine if they are aliases to each other. *** The merge process is now explicit and more flexible. Previously, all fields of the resolved keyword were unconditionally used for the alias (the pointer to an alias keyword structure was replaced by one to the aliased keyword's one). Now, field 'final_kw' on the alias keyword will store a pointer to the aliased keyword structure (and not only its name, as a subsequent commit will need the structure address). Fields 'header', 'field' and 'flag' are taken from the aliased keyword if they have default values (NULL or 0), else the alias' values prevail. This allows an alias to override one or more of these fields. All fields after 'oproc', because they describe the information to display consistently with each other, are always taken from the aliased keyword. merge_alias() checks that the values of these fields in the alias keyword structure are unspecified (NULL, or some neutral value like 0 and UNSPEC). While here, parsefmt() was reworked to avoid any direct recursion and the break-up/recombination steps that were used when processing aliases. The latter was due to the mutual recursion with findvar() and its odd-for-that-purpose signature. findvar() has been removed in the process. Simplification of parsefmt() also allows to be more precise with the errors reported (in particular, the case of an empty keyword with a specific header would just be reported as a "keyword not found" message). While here, introduce the check_keywords() function, performing sanity checks on the declared keywords, currently only validating that they are declared in alphabetical order. As for resolve_aliases(), this function is called at startup on PS_CHECK_KEYWORDS, else it is available to be called from a debugger. Ideally, alias resolution should be done at compile time. In practice, it seems doing so at runtime was never a problem (there are only a few aliases compared to all available keywords, and there's currently at most one level of aliasing). With the changes here, it seems very unlikely to become one even if many more keywords, aliases or aliasing levels are added. MFC after: 3 days Sponsored by: The FreeBSD Foundation --- bin/ps/Makefile | 3 + bin/ps/extern.h | 2 + bin/ps/keyword.c | 244 +++++++++++++++++++++++++++++++++++++++---------------- bin/ps/ps.c | 7 ++ bin/ps/ps.h | 21 +++-- 5 files changed, 200 insertions(+), 77 deletions(-) diff --git a/bin/ps/Makefile b/bin/ps/Makefile index 71973b34dd24..94a6b07757f0 100644 --- a/bin/ps/Makefile +++ b/bin/ps/Makefile @@ -3,5 +3,8 @@ PROG= ps SRCS= fmt.c keyword.c nlist.c print.c ps.c LIBADD= m kvm jail xo +.ifdef PS_CHECK_KEYWORDS +CFLAGS+=-DPS_CHECK_KEYWORDS +.endif .include diff --git a/bin/ps/extern.h b/bin/ps/extern.h index 45b5969f3911..93f1b018a88c 100644 --- a/bin/ps/extern.h +++ b/bin/ps/extern.h @@ -43,6 +43,7 @@ extern struct velisthead varlist; __BEGIN_DECLS char *arguments(KINFO *, VARENT *); +void check_keywords(void); char *command(KINFO *, VARENT *); char *cputime(KINFO *, VARENT *); char *cpunum(KINFO *, VARENT *); @@ -73,6 +74,7 @@ void printheader(void); char *priorityr(KINFO *, VARENT *); char *egroupname(KINFO *, VARENT *); char *rgroupname(KINFO *, VARENT *); +void resolve_aliases(void); char *runame(KINFO *, VARENT *); char *rvar(KINFO *, VARENT *); void showkey(void); diff --git a/bin/ps/keyword.c b/bin/ps/keyword.c index 8fc42d8675e5..32a6992ea51d 100644 --- a/bin/ps/keyword.c +++ b/bin/ps/keyword.c @@ -36,6 +36,8 @@ #include #include +#include +#include #include #include #include @@ -44,7 +46,6 @@ #include "ps.h" -static VAR *findvar(char *, struct velisthead *, int, char **header); static int vcmp(const void *, const void *); /* Compute offset in common structures. */ @@ -229,6 +230,120 @@ static VAR keywords[] = { static const size_t known_keywords_nb = nitems(keywords); +/* + * Sanity checks on declared keywords. + * + * Checks specific to aliases are done in resolve_alias() instead. + * + * Currently, only checks that keywords are alphabetically ordered by their + * names. More checks could be added, such as the absence of type (UNSPEC), + * 'fmt' (NULL) when the output routine is not kval()/rval(). + * + * Called from main() on PS_CHECK_KEYWORDS, else available when debugging. + */ +void +check_keywords(void) +{ + const VAR *k, *next_k; + bool order_violated = false; + + k = &keywords[0]; + for (size_t i = 1; i < known_keywords_nb; ++i) { + next_k = &keywords[i]; + if (vcmp(k, next_k) >= 0) { + xo_warnx("keywords bad order: '%s' followed by '%s'", + k->name, next_k->name); + order_violated = true; + } + k = next_k; + } + if (order_violated) + /* Must be the case as we rely on bsearch() + vcmp(). */ + xo_errx(2, "keywords are not in ascending order " + "(internal error)"); +} + +static void +alias_errx(const char *const name, const char *const what) +{ + xo_errx(2, "alias keyword '%s' specifies %s (internal error)", + name, what); +} + +static void +merge_alias(VAR *const k, VAR *const tgt) +{ + if ((tgt->flag & RESOLVED_ALIAS) != 0) + k->final_kw = tgt->final_kw; + else { + k->final_kw = tgt; + assert(tgt->aliased == NULL); + } + +#define MERGE_IF_SENTINEL(field, sentinel) do { \ + if (k->field == sentinel) \ + k->field = tgt->field; \ +} while (0) + + MERGE_IF_SENTINEL(header, NULL); + MERGE_IF_SENTINEL(field, NULL); + /* If NOINHERIT is present, no merge occurs. */ + MERGE_IF_SENTINEL(flag, 0); + +#undef MERGE_IF_SENTINEL + + /* We also check that aliases don't specify things they should not. */ +#define MERGE_CHECK_SENTINEL(field, sentinel, field_descr) do { \ + if (k->field != sentinel) \ + alias_errx(k->name, field_descr); \ + k->field = tgt->field; \ +} while (0); + + MERGE_CHECK_SENTINEL(oproc, NULL, "an output routine"); + MERGE_CHECK_SENTINEL(off, 0, "a structure offset"); + MERGE_CHECK_SENTINEL(type, UNSPEC, "a different type than UNSPEC"); + MERGE_CHECK_SENTINEL(fmt, NULL, "a printf format"); + +#undef MERGE_CHECK_SENTINEL +} + +static void +resolve_alias(VAR *const k) +{ + VAR *t, key; + + if ((k->flag & RESOLVED_ALIAS) != 0 || k->aliased == NULL) + return; + + if ((k->flag & RESOLVING_ALIAS) != 0) + xo_errx(2, "cycle when resolving alias keyword '%s'", k->name); + k->flag |= RESOLVING_ALIAS; + + key.name = k->aliased; + t = bsearch(&key, keywords, known_keywords_nb, sizeof(VAR), vcmp); + if (t == NULL) + xo_errx(2, "unknown target '%s' for keyword alias '%s'", + k->aliased, k->name); + + resolve_alias(t); + merge_alias(k, t); + + k->flag &= ~RESOLVING_ALIAS; + k->flag |= RESOLVED_ALIAS; +} + +/* + * Resolve all aliases immediately. + * + * Called from main() on PS_CHECK_KEYWORDS, else available when debugging. + */ +void +resolve_aliases(void) +{ + for (size_t i = 0; i < known_keywords_nb; ++i) + resolve_alias(&keywords[i]); +} + void showkey(void) { @@ -261,30 +376,56 @@ void parsefmt(const char *p, struct velisthead *const var_list, const int user) { - char *tempstr, *tempstr1; + char *copy, *cp; + char *hdr_p, sep; + size_t sep_idx; + VAR *v, key; + struct varent *vent; + + cp = copy = strdup(p); + if (copy == NULL) + xo_err(1, "strdup"); -#define FMTSEP " \t,\n" - tempstr1 = tempstr = strdup(p); - while (tempstr && *tempstr) { - char *cp, *hp; - VAR *v; - struct varent *vent; + sep = cp[0]; /* We only care if it's 0 or not here. */ + sep_idx = -1; + while (sep != '\0') { + cp += sep_idx + 1; /* * If an item contains an equals sign, it specifies a column * header, may contain embedded separator characters and * is always the last item. */ - if (tempstr[strcspn(tempstr, "="FMTSEP)] != '=') - while ((cp = strsep(&tempstr, FMTSEP)) != NULL && - *cp == '\0') - /* void */; - else { - cp = tempstr; - tempstr = NULL; + sep_idx = strcspn(cp, "= \t,\n"); + sep = cp[sep_idx]; + cp[sep_idx] = 0; + if (sep == '=') { + hdr_p = cp + sep_idx + 1; + sep = '\0'; /* No more keywords. */ + } else + hdr_p = NULL; + + /* At this point, '*cp' is '\0' iff 'sep_idx' is 0. */ + if (*cp == '\0') { + /* + * Empty keyword. Skip it, and silently unless some + * header has been specified. + */ + if (hdr_p != NULL) + xo_warnx("empty keyword with header '%s'", + hdr_p); + continue; } - if (cp == NULL || !(v = findvar(cp, var_list, user, &hp))) + + /* Find the keyword. */ + key.name = cp; + v = bsearch(&key, keywords, + known_keywords_nb, sizeof(VAR), vcmp); + if (v == NULL) { + xo_warnx("%s: keyword not found", cp); + eval = 1; continue; + } if (!user) { /* * If the user is NOT adding this field manually, @@ -295,19 +436,31 @@ parsefmt(const char *p, struct velisthead *const var_list, if (vent != NULL) continue; } + +#ifndef PS_CHECK_KEYWORDS + /* + * On PS_CHECK_KEYWORDS, this is not necessary as all aliases + * are resolved at startup in main() by calling + * resolve_aliases(). + */ + resolve_alias(v); +#endif + if ((vent = malloc(sizeof(struct varent))) == NULL) xo_errx(1, "malloc failed"); vent->header = v->header; - if (hp) { - hp = strdup(hp); - if (hp) - vent->header = hp; + if (hdr_p) { + hdr_p = strdup(hdr_p); + if (hdr_p) + vent->header = hdr_p; } vent->width = strlen(vent->header); vent->var = v; STAILQ_INSERT_TAIL(var_list, vent, next_ve); } - free(tempstr1); + + free(copy); + if (STAILQ_EMPTY(var_list)) { xo_warnx("no valid keywords; valid keywords:"); showkey(); @@ -315,55 +468,6 @@ parsefmt(const char *p, struct velisthead *const var_list, } } -static VAR * -findvar(char *p, struct velisthead *const var_list, int user, char **header) -{ - size_t rflen; - VAR *v, key; - char *hp, *realfmt; - - hp = strchr(p, '='); - if (hp) - *hp++ = '\0'; - - key.name = p; - v = bsearch(&key, keywords, known_keywords_nb, sizeof(VAR), vcmp); - - if (v && v->aliased) { - /* - * If the user specified an alternate-header for this - * (aliased) format-name, then we need to copy that - * alternate-header when making the recursive call to - * process the alias. - */ - if (hp == NULL) - parsefmt(v->aliased, var_list, user); - else { - /* - * XXX - This processing will not be correct for - * any alias which expands into a list of format - * keywords. Presently there are no aliases - * which do that. - */ - rflen = strlen(v->aliased) + strlen(hp) + 2; - realfmt = malloc(rflen); - if (realfmt == NULL) - xo_errx(1, "malloc failed"); - snprintf(realfmt, rflen, "%s=%s", v->aliased, hp); - parsefmt(realfmt, var_list, user); - free(realfmt); - } - return ((VAR *)NULL); - } - if (!v) { - xo_warnx("%s: keyword not found", p); - eval = 1; - } - if (header) - *header = hp; - return (v); -} - static int vcmp(const void *a, const void *b) { diff --git a/bin/ps/ps.c b/bin/ps/ps.c index 10b3322dc269..a0b7297afe7b 100644 --- a/bin/ps/ps.c +++ b/bin/ps/ps.c @@ -203,6 +203,13 @@ main(int argc, char *argv[]) pidmax_init(); +#ifdef PS_CHECK_KEYWORDS + /* Check for obvious problems in the keywords array. */ + check_keywords(); + /* Resolve all aliases at start to spot errors. */ + resolve_aliases(); +#endif + all = descendancy = _fmt = nselectors = optfatal = 0; prtheader = showthreads = wflag = xkeep_implied = 0; xkeep = -1; /* Neither -x nor -X. */ diff --git a/bin/ps/ps.h b/bin/ps/ps.h index f52f67f50151..9b85972e688c 100644 --- a/bin/ps/ps.h +++ b/bin/ps/ps.h @@ -63,20 +63,27 @@ typedef struct varent { } VARENT; STAILQ_HEAD(velisthead, varent); +struct var; +typedef struct var VAR; /* Structure representing one available keyword. */ -typedef struct var { +struct var { const char *name; /* name(s) of variable */ union { + /* Valid field depends on RESOLVED_ALIAS' presence. */ const char *aliased; /* keyword this one is an alias to */ + const VAR *final_kw; /* final aliased keyword */ }; const char *header; /* default header */ const char *field; /* xo field name */ -#define COMM 0x01 /* needs exec arguments and environment (XXX) */ -#define LJUST 0x02 /* left adjust on output (trailing blanks) */ -#define USER 0x04 /* needs user structure */ -#define INF127 0x10 /* values >127 displayed as 127 */ +#define COMM 0x01 /* needs exec arguments and environment (XXX) */ +#define LJUST 0x02 /* left adjust on output (trailing blanks) */ +#define USER 0x04 /* needs user structure */ +#define INF127 0x10 /* values >127 displayed as 127 */ +#define NOINHERIT 0x1000 /* Don't inherit flags from aliased keyword. */ +#define RESOLVING_ALIAS 0x10000 /* Used transiently to resolve aliases. */ +#define RESOLVED_ALIAS 0x20000 /* Mark that an alias has been resolved. */ u_int flag; - /* output routine */ + /* output routine */ char *(*oproc)(struct kinfo *, struct varent *); /* * The following (optional) elements are hooks for passing information @@ -86,6 +93,6 @@ typedef struct var { size_t off; /* offset in structure */ enum type type; /* type of element */ const char *fmt; /* printf format (depends on output routine) */ -} VAR; +}; #include "extern.h"