iostat: cap wait/interval at 100 million seconds

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

iostat: cap wait/interval at 100 million seconds

Scott Cheloha
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.c 23 Oct 2015 08:21:27 -0000 1.39
+++ usr.sbin/iostat/iostat.c 9 Feb 2018 23:39:15 -0000
@@ -146,9 +146,9 @@ main(int argc, char *argv[])
  todo |= SHOW_TTY;
  break;
  case 'w':
- interval = strtonum(optarg, 1, INT_MAX, &errstr);
+ interval = strtonum(optarg, 1, 100000000, &errstr);
  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, &errstr);
+ interval = strtonum(*argv, 1, 100000000, &errstr);
  if (errstr)
  errx(1, "interval is %s", errstr);
  if (*++argv) {

Reply | Threaded
Open this post in threaded view
|

Re: iostat: cap wait/interval at 100 million seconds

Theo Buehler-3
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.

Reply | Threaded
Open this post in threaded view
|

Re: iostat: cap wait/interval at 100 million seconds

Scott Cheloha
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