The branch main has been updated by kevans:
URL:
https://cgit.FreeBSD.org/src/commit/?id=5f72125339b1d14d1b04329ac561354f5e8133fe
commit 5f72125339b1d14d1b04329ac561354f5e8133fe
Author: Kyle Evans <kev...@freebsd.org>
AuthorDate: 2025-08-09 16:00:31 +0000
Commit: Kyle Evans <kev...@freebsd.org>
CommitDate: 2025-08-09 16:00:31 +0000
top: improve sort field storage/lookup
Switch up comparator mapping to avoid these kinds of errors, use a
simple array of (name, comparator) pairs rather than having to maintain
entries in two separate arrays that must have matching indices.
Reviewed by: obiwac
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D37083
---
usr.bin/top/machine.c | 155 ++++++++++++++++++++++++++++++++++++++++----------
usr.bin/top/machine.h | 7 ++-
usr.bin/top/top.c | 29 ++++------
usr.bin/top/utils.c | 21 -------
usr.bin/top/utils.h | 1 -
5 files changed, 142 insertions(+), 71 deletions(-)
diff --git a/usr.bin/top/machine.c b/usr.bin/top/machine.c
index 8c035b5df383..1f70ee9281e8 100644
--- a/usr.bin/top/machine.c
+++ b/usr.bin/top/machine.c
@@ -190,15 +190,6 @@ static int pageshift; /* log base 2 of the
pagesize */
#define ki_swap(kip) \
((kip)->ki_swrss > (kip)->ki_rssize ? (kip)->ki_swrss - (kip)->ki_rssize :
0)
-/*
- * Sorting orders. The first element is the default.
- */
-static const char *ordernames[] = {
- "cpu", "size", "res", "time", "pri", "threads",
- "total", "read", "write", "fault", "vcsw", "ivcsw",
- "jid", "swap", "pid", NULL
-};
-
/* Per-cpu time states */
static int maxcpu;
static int maxid;
@@ -214,6 +205,18 @@ static int *pcpu_cpu_states;
static int battery_units;
static int battery_life;
+static int compare_cpu(const void *a, const void *b);
+static int compare_size(const void *a, const void *b);
+static int compare_res(const void *a, const void *b);
+static int compare_time(const void *a, const void *b);
+static int compare_prio(const void *a, const void *b);
+static int compare_threads(const void *a, const void *b);
+static int compare_iototal(const void *a, const void *b);
+static int compare_ioread(const void *a, const void *b);
+static int compare_iowrite(const void *a, const void *b);
+static int compare_iofault(const void *a, const void *b);
+static int compare_vcsw(const void *a, const void *b);
+static int compare_ivcsw(const void *a, const void *b);
static int compare_swap(const void *a, const void *b);
static int compare_jid(const void *a, const void *b);
static int compare_pid(const void *a, const void *b);
@@ -225,6 +228,77 @@ static void update_layout(void);
static int find_uid(uid_t needle, int *haystack);
static int cmd_matches(struct kinfo_proc *, const char *);
+/*
+ * Sorting orders. The first element is the default.
+ */
+
+typedef int (compare_fn)(const void *arg1, const void *arg2);
+static const struct sort_info {
+ const char *si_name;
+ compare_fn *si_compare;
+} sortdata[] = {
+ {
+ .si_name = "cpu",
+ .si_compare = &compare_cpu,
+ },
+ {
+ .si_name = "size",
+ .si_compare = &compare_size,
+ },
+ {
+ .si_name = "res",
+ .si_compare = &compare_res,
+ },
+ {
+ .si_name = "time",
+ .si_compare = &compare_time,
+ },
+ {
+ .si_name = "pri",
+ .si_compare = &compare_prio,
+ },
+ {
+ .si_name = "threads",
+ .si_compare = &compare_threads,
+ },
+ {
+ .si_name = "total",
+ .si_compare = &compare_iototal,
+ },
+ {
+ .si_name = "read",
+ .si_compare = &compare_ioread,
+ },
+ {
+ .si_name = "write",
+ .si_compare = &compare_iowrite,
+ },
+ {
+ .si_name = "fault",
+ .si_compare = &compare_iofault,
+ },
+ {
+ .si_name = "vcsw",
+ .si_compare = &compare_vcsw,
+ },
+ {
+ .si_name = "ivcsw",
+ .si_compare = &compare_ivcsw,
+ },
+ {
+ .si_name = "jid",
+ .si_compare = &compare_jid,
+ },
+ {
+ .si_name = "swap",
+ .si_compare = &compare_swap,
+ },
+ {
+ .si_name = "pid",
+ .si_compare = &compare_pid,
+ },
+};
+
static int
find_uid(uid_t needle, int *haystack)
{
@@ -353,7 +427,6 @@ machine_init(struct statics *statics)
statics->swap_names = swapnames;
else
statics->swap_names = NULL;
- statics->order_names = ordernames;
/* Allocate state for per-CPU stats. */
GETSYSCTL("kern.smp.maxcpus", maxcpu);
@@ -742,7 +815,7 @@ static struct handle handle;
void *
get_process_info(struct system_info *si, struct process_select *sel,
- int (*compare)(const void *, const void *))
+ const struct sort_info *sort_info)
{
int i;
int total_procs;
@@ -753,6 +826,9 @@ get_process_info(struct system_info *si, struct
process_select *sel,
struct kinfo_proc **prefp;
struct kinfo_proc *pp;
struct timespec previous_proc_uptime;
+ compare_fn *compare;
+
+ compare = sort_info->si_compare;
/*
* If thread state was toggled, don't cache the previous processes.
@@ -899,6 +975,43 @@ get_process_info(struct system_info *si, struct
process_select *sel,
return (&handle);
}
+/*
+ * Returns the sort info associated with the specified order. Currently,
that's
+ * really only the comparator that we'll later use. Specifying a NULL
ordername
+ * will return the default comparator.
+ */
+const struct sort_info *
+get_sort_info(const char *ordername)
+{
+ const struct sort_info *info;
+ size_t idx;
+
+ if (ordername == NULL)
+ return (&sortdata[0]);
+
+ for (idx = 0; idx < nitems(sortdata); idx++) {
+ info = &sortdata[idx];
+
+ if (strcmp(info->si_name, ordername) == 0)
+ return (info);
+ }
+
+ return (NULL);
+}
+
+void
+dump_sort_names(FILE *fp)
+{
+ const struct sort_info *info;
+ size_t idx;
+
+ for (idx = 0; idx < nitems(sortdata); idx++) {
+ info = &sortdata[idx];
+
+ fprintf(fp, " %s", info->si_name);
+ }
+}
+
static int
cmd_matches(struct kinfo_proc *proc, const char *term)
{
@@ -1559,26 +1672,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
return (flp2 - flp1);
}
-int (*compares[])(const void *arg1, const void *arg2) = {
- compare_cpu,
- compare_size,
- compare_res,
- compare_time,
- compare_prio,
- compare_threads,
- compare_iototal,
- compare_ioread,
- compare_iowrite,
- compare_iofault,
- compare_vcsw,
- compare_ivcsw,
- compare_jid,
- compare_swap,
- compare_pid,
- NULL
-};
-
-
static int
swapmode(int *retavail, int *retfree)
{
diff --git a/usr.bin/top/machine.h b/usr.bin/top/machine.h
index 57f2846cdba5..b4f2f1a8bd50 100644
--- a/usr.bin/top/machine.h
+++ b/usr.bin/top/machine.h
@@ -89,10 +89,15 @@ void get_system_info(struct system_info *si);
int machine_init(struct statics *statics);
/* non-int routines typically used by the machine dependent module */
+struct sort_info;
+
extern struct process_select ps;
void *
get_process_info(struct system_info *si, struct process_select *sel,
- int (*compare)(const void *, const void *));
+ const struct sort_info *);
+
+const struct sort_info *get_sort_info(const char *name);
+void dump_sort_names(FILE *fp);
#endif /* MACHINE_H */
diff --git a/usr.bin/top/top.c b/usr.bin/top/top.c
index 856ad838dc1c..f0458a4037af 100644
--- a/usr.bin/top/top.c
+++ b/usr.bin/top/top.c
@@ -252,7 +252,7 @@ main(int argc, const char *argv[])
char no_command = 1;
struct timeval timeout;
char *order_name = NULL;
- int order_index = 0;
+ const struct sort_info *sort_info = NULL;
fd_set readfds;
char *nptr;
@@ -505,21 +505,18 @@ main(int argc, const char *argv[])
/* determine sorting order index, if necessary */
if (order_name != NULL)
{
- if ((order_index = string_index(order_name, statics.order_names)) == -1)
- {
- const char * const *pp;
-
+ if ((sort_info = get_sort_info(order_name)) == NULL) {
warnx("'%s' is not a recognized sorting order.", order_name);
fprintf(stderr, "\tTry one of these:");
- pp = statics.order_names;
- while (*pp != NULL)
- {
- fprintf(stderr, " %s", *pp++);
- }
+ dump_sort_names(stderr);
fputc('\n', stderr);
exit(1);
}
}
+ else
+ {
+ sort_info = get_sort_info(NULL);
+ }
/* initialize termcap */
init_termcap(interactive);
@@ -602,17 +599,13 @@ restart:
while ((displays == -1) || (displays-- > 0))
{
- int (*compare)(const void * const, const void * const);
-
/* get the current stats */
get_system_info(&system_info);
- compare = compares[order_index];
-
/* get the current set of processes */
processes =
- get_process_info(&system_info, &ps, compare);
+ get_process_info(&system_info, &ps, sort_info);
/* display the load averages */
(*d_loadave)(system_info.last_pid,
@@ -1047,7 +1040,9 @@ restart:
"Order to sort: ");
if (readline(tempbuf2, sizeof(tempbuf2), false)
> 0)
{
- if ((i = string_index(tempbuf2,
statics.order_names)) == -1)
+ const struct sort_info *new_sort_info;
+
+ if ((new_sort_info = get_sort_info(tempbuf2))
== NULL)
{
new_message(MT_standout,
" %s: unrecognized sorting
order", tempbuf2);
@@ -1055,7 +1050,7 @@ restart:
}
else
{
- order_index = i;
+ sort_info = new_sort_info;
}
putchar('\r');
}
diff --git a/usr.bin/top/utils.c b/usr.bin/top/utils.c
index cde89a211b53..a8ddb3eb63a0 100644
--- a/usr.bin/top/utils.c
+++ b/usr.bin/top/utils.c
@@ -116,27 +116,6 @@ digits(int val)
return(cnt);
}
-/*
- * string_index(string, array) - find string in array and return index
- */
-
-int
-string_index(const char *string, const char * const *array)
-{
- size_t i = 0;
-
- while (*array != NULL)
- {
- if (strcmp(string, *array) == 0)
- {
- return(i);
- }
- array++;
- i++;
- }
- return(-1);
-}
-
/*
* argparse(line, cntp) - parse arguments in string "line", separating them
* out into an argv-like array, and setting *cntp to the number of
diff --git a/usr.bin/top/utils.h b/usr.bin/top/utils.h
index a730e339d200..bbc63803b6e1 100644
--- a/usr.bin/top/utils.h
+++ b/usr.bin/top/utils.h
@@ -19,6 +19,5 @@ const char **argparse(char *, int *);
long percentages(int, int *, long *, long *, long *);
const char *format_time(long);
char *format_k(int64_t);
-int string_index(const char *string, const char * const *array);
int find_pid(pid_t pid);
