On Mon, 2021-01-25 at 10:50 +0100, Martijn van Duren wrote:
> 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@
>
That one was a little too rushed... It overlooked the internal "s"
command and some minor additional testing shows that currently
large delays are accepted and completely broken because of useconds_t
overflow. I couldn't find something like USECONDS_MAX, so I took the
next best thing: UINT32_MAX, where useconds_t is based on.
This change limits delays to 4294 seconds, which already is the
only reasonable value that would work for systat.
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 10:16:34 -0000
@@ -40,9 +40,11 @@
#include <errno.h>
#include <fcntl.h>
#include <limits.h>
+#include <math.h>
#include <netdb.h>
#include <signal.h>
#include <stdio.h>
+#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
@@ -73,6 +75,7 @@ char uloadbuf[TIMEPOS];
int ucount(void);
void usage(void);
+double strtodnum(const char *, double, double, const char **);
/* command prompt */
@@ -323,9 +326,14 @@ void
cmd_delay(const char *buf)
{
double del;
- del = atof(buf);
+ const char *errstr;
- if (del > 0) {
+ if (buf[0] == '\0')
+ return;
+ del = strtodnum(buf, 0, UINT32_MAX / 1000000, &errstr);
+ if (errstr != NULL)
+ error("s: \"%s\": delay value is %s", buf, errstr);
+ else {
udelay = (useconds_t)(del * 1000000);
gotsig_alarm = 1;
naptime = del;
@@ -414,6 +422,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 +525,11 @@ main(int argc, char *argv[])
nflag = 1;
break;
case 's':
- delay = atof(optarg);
- if (delay <= 0)
- delay = 5;
+ delay = strtodnum(optarg, 0, UINT32_MAX / 1000000,
+ &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 +549,14 @@ main(int argc, char *argv[])
argv += optind;
if (argc == 1) {
- double del = atof(argv[0]);
- if (del == 0)
+ delay = strtodnum(argv[0], 0, UINT32_MAX / 1000000, &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, UINT32_MAX / 1000000, &errstr);
+ if (errstr != NULL)
+ errx(1, "-s \"%s\": delay value is %s", optarg, errstr);
}
udelay = (useconds_t)(delay * 1000000.0);
