Module Name:    src
Committed By:   kre
Date:           Mon Mar 27 23:20:13 UTC 2023

Modified Files:
        src/usr.bin/systat: disks.c

Log Message:
Undo previous "restore lost break" and fix the issue that seems to
have been intending to correct properly (or at least, more rationally).

This makes adding/deleting drives for "vmstat" (and "iostat", though
the man page doesn't say that it also works there) using the :display
:ignore and :drive sub-commands work sensibly using fnmatch()
rather than what was there most of the time since it was added.

Previously ":ignore dk*" would ignore the first dk (wedge) found in
the system (whether or not it was ignored already) and that was it.
[":ignore dk* dk*" would just do that twice.]

Now the same command will ignore all dk* drives (all wedges), which
is what I would anticipate almost anyone would expect it to do.

Similarly for ":display" (to add drives) and ":drives" to explictly
list particular ones.

When the fnmatch() code was added, almost 9 and a half years ago now,
it was almost correct - except always resulted in an error occurring
(though that was little more than a minor inconvenience).

That was "fixed" 5 months later (9 years and almost a month ago now)
with the cvs log message "restore lost break" - which was absolutely
the wrong thing to do (the break was fine when no patterns were used,
and so a name could only ever match one drive - but wrong when the
whole point is to match many).

Somehow in the past 9+ years, no-one noticed that this functionality
had been rendered almost useless.

While here, fix a related problem ... just above I referred to the
error that occurred as a "minor inconvenience" - that's because while
an error would be shown on the screen, it would then immediately be
removed again, an observant user might notice the quick flash, but
that would be it.

Handle that by detecting whether any changes are actually made, and
don't go completely redrawing the screen (removing the error message
that was just placed there) if there is no point.   This doesn't
entirely fix this problem, as if we do

        :drives foo wd*

and there is no "foo" drive in the system, we'd get an error message
from that, but adding the wd* drives (assuming there are some, of course)
counts as a change, so that error message will still not last very long.
The order of that command line makes no difference, it isn't that wd drives
were found after foo wasn't, but that the whole line matched (at least one)
drive (and changed its state - for the "drives" command, that is equivalent
to matched) - but also contained an entry which did not match at all.
That's a harder problem to fix.

No pullups planned, as no-one seems to mind how it has been all this time.


To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/usr.bin/systat/disks.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/systat/disks.c
diff -u src/usr.bin/systat/disks.c:1.19 src/usr.bin/systat/disks.c:1.20
--- src/usr.bin/systat/disks.c:1.19	Sat Mar  8 20:51:20 2014
+++ src/usr.bin/systat/disks.c	Mon Mar 27 23:20:13 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: disks.c,v 1.19 2014/03/08 20:51:20 jdc Exp $	*/
+/*	$NetBSD: disks.c,v 1.20 2023/03/27 23:20:13 kre Exp $	*/
 
 /*-
  * Copyright (c) 1980, 1992, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)disks.c	8.1 (Berkeley) 6/6/93";
 #endif
-__RCSID("$NetBSD: disks.c,v 1.19 2014/03/08 20:51:20 jdc Exp $");
+__RCSID("$NetBSD: disks.c,v 1.20 2023/03/27 23:20:13 kre Exp $");
 #endif /* not lint */
 
 #include <ctype.h>
@@ -71,6 +71,7 @@ disks_drives(char *args)
 	if (args) {
 		for (i = 0; i < ndrive; i++)
 			drv_select[i] = 0;
+		labels();
 		disks_add(args);
 	} else {
 		move(CMDLINE, 0);
@@ -85,6 +86,8 @@ drvselect(char *args, int truefalse, int
 {
 	char *cp;
 	size_t i;
+	int matched;
+	int changed = 0;
 
 	cp = strchr(args, '\n');
 	if (cp)
@@ -99,15 +102,20 @@ drvselect(char *args, int truefalse, int
 			*cp++ = '\0';
 		if (cp - args == 0)
 			break;
+		matched = 0;
 		for (i = 0; i < ndrive; i++)
 			if (fnmatch(args, dr_name[i], 0) == 0) {
+				if (selections[i] != truefalse)
+					changed = 1;
 				selections[i] = truefalse;
-				break;
+				matched = 1;
 			}
-		if (i >= ndrive)
+		if (matched == 0)
 			error("%s: unknown drive", args);
 		args = cp;
 	}
-	labels();
-	display(0);
+	if (changed) {
+		labels();
+		display(0);
+	}
 }

Reply via email to