git: aa9c42003b5d - stable/14 - ps(1): Aliases: Resolve once, merge specifications
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 01 May 2025 19:51:36 UTC
The branch stable/14 has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=aa9c42003b5d3570d34255576a6a548e89c5318c
commit aa9c42003b5d3570d34255576a6a548e89c5318c
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2025-04-25 13:20:08 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2025-05-01 19:37:04 +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
(cherry picked from commit 193e68d5174ffde70acc427f9d830797df9a13cb)
---
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 6d47b91e662e..120bf45962b0 100644
--- a/bin/ps/Makefile
+++ b/bin/ps/Makefile
@@ -12,5 +12,8 @@ SRCS= fmt.c keyword.c nlist.c print.c ps.c
#
CFLAGS+=-DLAZY_PS
LIBADD= m kvm jail xo
+.ifdef PS_CHECK_KEYWORDS
+CFLAGS+=-DPS_CHECK_KEYWORDS
+.endif
.include <bsd.prog.mk>
diff --git a/bin/ps/extern.h b/bin/ps/extern.h
index 94140f26d435..64d081484ace 100644
--- a/bin/ps/extern.h
+++ b/bin/ps/extern.h
@@ -45,6 +45,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 *);
@@ -75,6 +76,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 154d26039cdb..35604465d04c 100644
--- a/bin/ps/keyword.c
+++ b/bin/ps/keyword.c
@@ -42,6 +42,8 @@ static char sccsid[] = "@(#)keyword.c 8.5 (Berkeley) 4/2/94";
#include <sys/sysctl.h>
#include <sys/user.h>
+#include <assert.h>
+#include <stdbool.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
@@ -50,7 +52,6 @@ static char sccsid[] = "@(#)keyword.c 8.5 (Berkeley) 4/2/94";
#include "ps.h"
-static VAR *findvar(char *, struct velisthead *, int, char **header);
static int vcmp(const void *, const void *);
/* Compute offset in common structures. */
@@ -235,6 +236,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)
{
@@ -267,30 +382,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;
-#define FMTSEP " \t,\n"
- tempstr1 = tempstr = strdup(p);
- while (tempstr && *tempstr) {
- char *cp, *hp;
- VAR *v;
- struct varent *vent;
+ cp = copy = strdup(p);
+ if (copy == NULL)
+ xo_err(1, "strdup");
+
+ 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,
@@ -301,19 +442,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();
@@ -321,55 +474,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 fe100170b7e2..ad69027baa71 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@@ -225,6 +225,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 7c4c5541b8eb..8a07586642c8 100644
--- a/bin/ps/ps.h
+++ b/bin/ps/ps.h
@@ -65,20 +65,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
@@ -88,6 +95,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"