Re: iostat: cap wait/interval at 100 million seconds
On Sat, Feb 10, 2018 at 01:12:59PM +1300, Theo Buehler wrote: > On Fri, Feb 09, 2018 at 05:41:34PM -0600, Scott Cheloha wrote: > > Hi, > > > > nanosleep(2) won't take more than 100 million seconds at a time, > > so we ought to cap interval when we read it in. Otherwise, an > > oversized interval argument causes iostat(8) to print as fast as > > it can. > > > > While here, "interval" is called "wait" in the documentation when > > the new syntax is in use, so we should call it that in the error > > message for the '-w' flag. > > > > ok? > > I'm wondering if it wouldn't be better to check nanonsleep's return > value and do err(1, "nanosleep"); if it fails. This way we don't have > to remember to modify iostat if we ever change nanosleep to support > longer timeouts. In this case we can't because nanosleep(2) doesn't interact with signals, and iostat(8) installs a signal handler. So if it's in the kernel for nanosleep when it gets SIGCONT, errno goes to EINTR and the program exits, which isn't desirable. We could check explicitly for EINVAL, but I think the strtonum(3) error messages are nicer for the user than the strerror(3) default: iostat: nanosleep: Invalid argument Especially for numeric input. So I don't think we should add the err(3) here. -- Scott Cheloha
Re: iostat: cap wait/interval at 100 million seconds
On Fri, Feb 09, 2018 at 05:41:34PM -0600, Scott Cheloha wrote: > Hi, > > nanosleep(2) won't take more than 100 million seconds at a time, > so we ought to cap interval when we read it in. Otherwise, an > oversized interval argument causes iostat(8) to print as fast as > it can. > > While here, "interval" is called "wait" in the documentation when > the new syntax is in use, so we should call it that in the error > message for the '-w' flag. > > ok? I'm wondering if it wouldn't be better to check nanonsleep's return value and do err(1, "nanosleep"); if it fails. This way we don't have to remember to modify iostat if we ever change nanosleep to support longer timeouts.
iostat: cap wait/interval at 100 million seconds
Hi,
nanosleep(2) won't take more than 100 million seconds at a time,
so we ought to cap interval when we read it in. Otherwise, an
oversized interval argument causes iostat(8) to print as fast as
it can.
While here, "interval" is called "wait" in the documentation when
the new syntax is in use, so we should call it that in the error
message for the '-w' flag.
ok?
--
Scott Cheloha
Index: usr.sbin/iostat/iostat.c
===
RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
retrieving revision 1.39
diff -u -p -r1.39 iostat.c
--- usr.sbin/iostat/iostat.c23 Oct 2015 08:21:27 - 1.39
+++ usr.sbin/iostat/iostat.c9 Feb 2018 23:39:15 -
@@ -146,9 +146,9 @@ main(int argc, char *argv[])
todo |= SHOW_TTY;
break;
case 'w':
- interval = strtonum(optarg, 1, INT_MAX, );
+ interval = strtonum(optarg, 1, 1, );
if (errstr)
- errx(1, "interval is %s", errstr);
+ errx(1, "wait is %s", errstr);
break;
case '?':
default:
@@ -419,7 +419,7 @@ selectdrives(char *argv[])
errx(1, "invalid interval or drive name: %s", *argv);
}
if (*argv) {
- interval = strtonum(*argv, 1, INT_MAX, );
+ interval = strtonum(*argv, 1, 1, );
if (errstr)
errx(1, "interval is %s", errstr);
if (*++argv) {
