git: cd768a840644 - main - ps(1): Remove not-explicitly-requested columns with duplicate data
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 28 Apr 2025 12:23:28 UTC
The branch main has been updated by olce:
URL: https://cgit.FreeBSD.org/src/commit/?id=cd768a840644ad55029ce9c3d41fc52b5045e0cc
commit cd768a840644ad55029ce9c3d41fc52b5045e0cc
Author: Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2025-03-10 20:20:43 +0000
Commit: Olivier Certner <olce@FreeBSD.org>
CommitDate: 2025-04-28 11:57:52 +0000
ps(1): Remove not-explicitly-requested columns with duplicate data
Before this change, when stacking up more columns in the display through
command-line options, if user requested to add some "canned" display
(through options '-j', '-l', '-u' or '-v'), columns in it that were
"duplicates" of already requested ones (meaning that they share the same
keyword, regardless of whether their headers have been customized) were
in the end omitted.
However, this mechanism did not work the other way around, i.e.,
requesting some canned display(s) first and then adding some columns
that are duplicates (through '-o' or '-O') would not remove them from
the canned display. Additionally, it did not take into account keyword
aliases (which also lead to displaying the same information).
This whole mechanism of removing columns from requested canned displays
when there are duplicates is useful in a number of scenarios:
1. When one wants the columns of a canned display, but with some of them
in a different order and at the edges of the bulk. This needs the
change here to move columns after the bulk (previously, only moving
them before the bulk would work).
2. To combine multiple canned displays to get more information without
repeating common columns. This part has been working before and
this behavior is unchanged.
3. In combination with requesting a canned display and additional
columns after it, ensure that a single COMMAND column appears at the
end of the display (to benefit from the fact that a last COMMAND
column can extend further to the right).
Point 2 above implies that, when multiple canned displays are requested,
we should keep the leftmost column with same keyword. However, columns
requested explicitly by '-o' have priority (as the natural extension of
point 1's behavior before this change), and in this case all matching
columns in canned displays must be suppressed.
To this end, when adding requested options to the display's list, we
stop trying to find an earlier matching column (which is incidentally
a O(n²) algorithm). Instead, we do a first pass over all requested
options once they have all been added to the display's list (in
scan_vars()). For each keyword, we note if it was requested at least
once explicitly (through '-o' or '-O'), in addition to setting
'needuser' and 'needcomm' (as before). Then, a second pass decides
whether to keep each column. A column is removed if it should not be
kept absolutely (i.e., it wasn't specified via '-o' or '-O') and there
is either a matching column that must be kept (determined during the
first pass), or we have seen one already (earlier canned displays take
precedence).
Matching columns are in fact not only those that have same keywords, but
also those that have keywords determined to be aliases to each other.
Some previous commits ensured that this determination is O(1) and in
practice just a few assembly instructions.
find_varentry() has been kept although its last caller has been removed
as next commit will reintroduce a call to it.
MFC after: 3 days
Relnotes: yes
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D49612
Differential Revision: https://reviews.freebsd.org/D49613 (manual page)
---
bin/ps/extern.h | 2 ++
bin/ps/keyword.c | 25 +++++++++--------
bin/ps/ps.1 | 23 +++++++---------
bin/ps/ps.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++------
bin/ps/ps.h | 2 ++
5 files changed, 102 insertions(+), 32 deletions(-)
diff --git a/bin/ps/extern.h b/bin/ps/extern.h
index 93f1b018a88c..bb7a21bbc8be 100644
--- a/bin/ps/extern.h
+++ b/bin/ps/extern.h
@@ -40,6 +40,7 @@ extern unsigned long mempages;
extern time_t now;
extern int showthreads, sumrusage, termwidth;
extern struct velisthead varlist;
+extern const size_t known_keywords_nb;
__BEGIN_DECLS
char *arguments(KINFO *, VARENT *);
@@ -55,6 +56,7 @@ VARENT *find_varentry(const char *);
const char *fmt_argv(char **, char *, char *, size_t);
double getpcpu(const KINFO *);
char *jailname(KINFO *, VARENT *);
+size_t aliased_keyword_index(const VAR *);
char *kvar(KINFO *, VARENT *);
char *label(KINFO *, VARENT *);
char *loginclass(KINFO *, VARENT *);
diff --git a/bin/ps/keyword.c b/bin/ps/keyword.c
index 32a6992ea51d..3c19912144ac 100644
--- a/bin/ps/keyword.c
+++ b/bin/ps/keyword.c
@@ -42,6 +42,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+
#include <libxo/xo.h>
#include "ps.h"
@@ -228,7 +229,18 @@ static VAR keywords[] = {
USHORT, "x"},
};
-static const size_t known_keywords_nb = nitems(keywords);
+const size_t known_keywords_nb = nitems(keywords);
+
+size_t
+aliased_keyword_index(const VAR *const v)
+{
+ const VAR *const fv = (v->flag & RESOLVED_ALIAS) == 0 ?
+ v : v->final_kw;
+ const size_t idx = fv - keywords;
+
+ assert(idx < known_keywords_nb);
+ return (idx);
+}
/*
* Sanity checks on declared keywords.
@@ -426,16 +438,6 @@ parsefmt(const char *p, struct velisthead *const var_list,
eval = 1;
continue;
}
- if (!user) {
- /*
- * If the user is NOT adding this field manually,
- * get on with our lives if this VAR is already
- * represented in the list.
- */
- vent = find_varentry(v->name);
- if (vent != NULL)
- continue;
- }
#ifndef PS_CHECK_KEYWORDS
/*
@@ -456,6 +458,7 @@ parsefmt(const char *p, struct velisthead *const var_list,
}
vent->width = strlen(vent->header);
vent->var = v;
+ vent->flags = user ? VE_KEEP : 0;
STAILQ_INSERT_TAIL(var_list, vent, next_ve);
}
diff --git a/bin/ps/ps.1 b/bin/ps/ps.1
index 412d6a5a599b..801b089820c7 100644
--- a/bin/ps/ps.1
+++ b/bin/ps/ps.1
@@ -140,11 +140,16 @@ and
designate specific predefined groups of columns, also called canned displays.
Appearance of any of these options inhibits the default display, replacing it
all with the requested columns, and in the order options are passed.
-The individual columns requested via a canned display option which have the same
-keyword as that of some column added by earlier options are not added again.
-This kind of automatic removal of duplicate keywords in canned displays is
-useful for slightly tweaking these displays without having to rebuild variants
-from scratch, e.g., using
+The individual columns requested via a canned display option that have the same
+keyword or an alias to that of some column added by an earlier canned display
+option, or by an explicit
+.Fl O
+or
+.Fl o
+option anywhere on the command line, are suppressed.
+This automatic removal of duplicate data in canned displays is useful for
+slightly tweaking these displays and/or combining multiple ones without having
+to rebuild variants from scratch, e.g., using only
.Fl o
options.
.Pp
@@ -1027,14 +1032,6 @@ elimination, contrary to those of canned displays.
Finally, columns requested through multiple occurences are not grouped together,
as one may naturally expect.
.Pp
-Automatic removal of duplicate columns from canned displays only works backwards
-at time of insertion, i.e., adding a new canned display will lead to checking
-columns before it but not those after it.
-Besides the inconsistency, this prevents relocating columns of canned displays
-further right, which can be useful, e.g., to relocate a column with the
-.Cm command
-keyword at end of display.
-.Pp
The
.Fl a
option has no effect if other options affecting the selection of processes are
diff --git a/bin/ps/ps.c b/bin/ps/ps.c
index a0b7297afe7b..ccbe40bbbc45 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@@ -82,6 +82,19 @@ int sumrusage; /* -S */
int termwidth; /* Width of the screen (0 == infinity). */
int showthreads; /* will threads be shown? */
+struct keyword_info {
+ /*
+ * Whether there is (at least) one column referencing this keyword that
+ * must be kept.
+ */
+#define KWI_HAS_MUST_KEEP_COLUMN (1 << 0)
+ /*
+ * Whether a column with such a keyword has been seen.
+ */
+#define KWI_SEEN (1 << 1)
+ u_int flags;
+};
+
struct velisthead varlist = STAILQ_HEAD_INITIALIZER(varlist);
static kvm_t *kd;
@@ -129,7 +142,8 @@ static void init_list(struct listinfo *, addelem_rtn, int, const char *);
static char *kludge_oldps_options(const char *, char *, const char *);
static int pscomp(const void *, const void *);
static void saveuser(KINFO *);
-static void scanvars(void);
+static void scan_vars(struct keyword_info *);
+static void remove_redundant_columns(struct keyword_info *);
static void pidmax_init(void);
static void usage(void);
@@ -166,6 +180,7 @@ main(int argc, char *argv[])
char fmtbuf[_POSIX2_LINE_MAX];
enum { NONE = 0, UP = 1, DOWN = 2, BOTH = 1 | 2 } directions = NONE;
struct { int traversed; int initial; } pid_count;
+ struct keyword_info *keywords_info;
(void) setlocale(LC_ALL, "");
time(&now); /* Used by routines in print.c. */
@@ -462,6 +477,22 @@ main(int argc, char *argv[])
if (!_fmt)
parsefmt(dfmt, &varlist, 0);
+ keywords_info = calloc(known_keywords_nb, sizeof(struct keyword_info));
+ if (keywords_info == NULL)
+ xo_errx(1, "malloc failed");
+ /*
+ * Scan requested variables, noting which structures are needed and
+ * which keywords are specified.
+ */
+ scan_vars(keywords_info);
+ /*
+ * Remove redundant columns from "canned" displays (see the callee's
+ * herald comment for more details).
+ */
+ remove_redundant_columns(keywords_info);
+ free(keywords_info);
+ keywords_info = NULL;
+
if (!all && nselectors == 0) {
uidlist.l.ptr = malloc(sizeof(uid_t));
if (uidlist.l.ptr == NULL)
@@ -471,12 +502,6 @@ main(int argc, char *argv[])
*uidlist.l.uids = getuid();
}
- /*
- * scan requested variables, noting what structures are needed,
- * and adjusting header widths as appropriate.
- */
- scanvars();
-
/*
* Get process list. If the user requested just one selector-
* option, then kvm_getprocs can be asked to return just those
@@ -1196,7 +1221,7 @@ find_varentry(const char *name)
}
static void
-scanvars(void)
+scan_vars(struct keyword_info *const keywords_info)
{
struct varent *vent;
const VAR *v;
@@ -1207,6 +1232,47 @@ scanvars(void)
needuser = 1;
if (v->flag & COMM)
needcomm = 1;
+ if ((vent->flags & VE_KEEP) != 0)
+ keywords_info[aliased_keyword_index(v)].flags |=
+ KWI_HAS_MUST_KEEP_COLUMN;
+ }
+}
+
+/*
+ * For each explicitly requested keyword, remove all the same keywords
+ * from "canned" displays. If the same keyword appears multiple times
+ * only in "canned displays", then keep the first (leftmost) occurence
+ * only (with the reasoning that columns requested first are the most
+ * important as their positions catch the eye more).
+ */
+static void
+remove_redundant_columns(struct keyword_info *const keywords_info)
+{
+ struct varent *prev_vent, *vent, *next_vent;
+
+ prev_vent = NULL;
+ STAILQ_FOREACH_SAFE(vent, &varlist, next_ve, next_vent) {
+ const VAR *const v = vent->var;
+ struct keyword_info *const kwi =
+ &keywords_info[aliased_keyword_index(v)];
+
+ /*
+ * If the current column is not marked as to absolutely keep,
+ * and we have either already output one with the same keyword
+ * or know we will output one later, remove it.
+ */
+ if ((vent->flags & VE_KEEP) == 0 &&
+ (kwi->flags & (KWI_HAS_MUST_KEEP_COLUMN | KWI_SEEN)) != 0) {
+ if (prev_vent == NULL)
+ STAILQ_REMOVE_HEAD(&varlist, next_ve);
+ else
+ STAILQ_REMOVE_AFTER(&varlist, prev_vent,
+ next_ve);
+ } else
+ prev_vent = vent;
+
+
+ kwi->flags |= KWI_SEEN;
}
}
diff --git a/bin/ps/ps.h b/bin/ps/ps.h
index 9b85972e688c..065a4c1f1c54 100644
--- a/bin/ps/ps.h
+++ b/bin/ps/ps.h
@@ -60,6 +60,8 @@ typedef struct varent {
const char *header;
const struct var *var;
u_int width;
+#define VE_KEEP (1 << 0)
+ uint16_t flags;
} VARENT;
STAILQ_HEAD(velisthead, varent);