On Mon, 2021-01-25 at 08:15 +0000, Nick Gasson wrote:
> Hi,
>
> I incorrectly ran "systat netstat -N" instead of "systat -N netstat" and
> got confused why it wasn't resolving host names. The -N gets parsed with
> atof to a 0s delay that is then clamped to 5s. The patch below instead
> prints an error if the delay cannot be parsed. I think the <= 0 case
> should also produce an error but I left the existing behaviour of
> setting it to 5s.
Your diff fails for the simple case of "systat", because of your added
else case. It's also not completely clear to the end user why things
fail.
I agree that failing loud on unparseable strings and just defaulting
back to the 5s is just stupid. So here's an updated diff that just
blatantly steals^Wborrows from strtonum.
OK?
martijn@
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/systat/main.c,v
retrieving revision 1.72
diff -u -p -r1.72 main.c
--- main.c 12 Jan 2020 20:51:08 -0000 1.72
+++ main.c 25 Jan 2021 09:48:44 -0000
@@ -40,6 +40,7 @@
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
+#include <math.h>
#include <netdb.h>
#include <signal.h>
#include <stdio.h>
@@ -414,6 +415,48 @@ gethz(void)
hz = cinf.hz;
}
+#define INVALID 1
+#define TOOSMALL 2
+#define TOOLARGE 3
+
+double
+strtodnum(const char *nptr, double minval, double maxval, const char **errstrp)
+{
+ double d = 0;
+ int error = 0;
+ char *ep;
+ struct errval {
+ const char *errstr;
+ int err;
+ } ev[4] = {
+ { NULL, 0 },
+ { "invalid", EINVAL },
+ { "too small", ERANGE },
+ { "too large", ERANGE },
+ };
+
+ ev[0].err = errno;
+ errno = 0;
+ if (minval > maxval) {
+ error = INVALID;
+ } else {
+ d = strtod(nptr, &ep);
+ if (nptr == ep || *ep != '\0')
+ error = INVALID;
+ else if ((d == -HUGE_VAL && errno == ERANGE) || d < minval)
+ error = TOOSMALL;
+ else if ((d == HUGE_VAL && errno == ERANGE) || d > maxval)
+ error = TOOLARGE;
+ }
+ if (errstrp != NULL)
+ *errstrp = ev[error].errstr;
+ errno = ev[error].err;
+ if (error)
+ d = 0;
+
+ return (d);
+}
+
int
main(int argc, char *argv[])
{
@@ -475,9 +518,9 @@ main(int argc, char *argv[])
nflag = 1;
break;
case 's':
- delay = atof(optarg);
- if (delay <= 0)
- delay = 5;
+ delay = strtodnum(optarg, 0, HUGE_VAL, &errstr);
+ if (errstr != NULL)
+ errx(1, "-s %s: delay value is %s", optarg,
errstr);
break;
case 'w':
rawwidth = strtonum(optarg, 1, MAX_LINE_BUF-1, &errstr);
@@ -497,16 +540,14 @@ main(int argc, char *argv[])
argv += optind;
if (argc == 1) {
- double del = atof(argv[0]);
- if (del == 0)
+ delay = strtodnum(argv[0], 0, HUGE_VAL, &errstr);
+ if (errstr != NULL)
viewstr = argv[0];
- else
- delay = del;
} else if (argc == 2) {
viewstr = argv[0];
- delay = atof(argv[1]);
- if (delay <= 0)
- delay = 5;
+ delay = strtodnum(optarg, 0, HUGE_VAL, &errstr);
+ if (errstr != NULL)
+ errx(1, "-s %s: delay value is %s", optarg, errstr);
}
udelay = (useconds_t)(delay * 1000000.0);
