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

@@ -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)
 {

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 */

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');
 				}

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

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);