On Sat, Apr 30, 2022 at 01:27:44AM +0200, Alexander Bluhm wrote:
> On Thu, Apr 28, 2022 at 08:54:02PM -0500, Scott Cheloha wrote:
> > On Thu, Sep 17, 2020 at 06:29:48PM -0500, Scott Cheloha wrote:
> > > [...]
> > >
> > > Using nanosleep(2) to print the stats periodically causes the period
> > > to drift.  If you use setitimer(2) it won't drift.
> > >
> > > ok?
> >
> > 19 month bump and rebase.
> >
> > I have updated the patch according to input from kn@.
> >
> > Once again, using nanosleep(2) here to print the stats periodically is
> > flawed.  The period will drift.  Using setitimer(2)/sigsuspend(2) is
> > better.
> >
> > While here:
> >
> > - We don't need the hundred million second upper bound anymore.  Just
> >   cap the wait at UINT_MAX seconds.
> >
> > - Use the idiomatic strtonum(3) error message format, it works here.
> >
> > ok?
> 
> I would prefer to block the alarm signal with sigprocmask(2) and
> only catch it during sigsuspend(2).  Although the timeout should
> only happen while we sleep, blocking signals while we don't expect
> them, gives me a better feeling.

Whenever I block signals, deraadt@ rises up out of the floorboards and
says "I hate masking signals, don't do that."

... but if millert@ is still fine with the attached patch, which does
sigprocmask(2), I'll go ahead with it.

> Please check the error code of signal(3).

Sure.

> otherwise diff looks good to me

Still look good?

Index: kstat.c
===================================================================
RCS file: /cvs/src/usr.bin/kstat/kstat.c,v
retrieving revision 1.9
diff -u -p -r1.9 kstat.c
--- kstat.c     22 Apr 2022 00:29:20 -0000      1.9
+++ kstat.c     3 May 2022 01:37:36 -0000
@@ -15,6 +15,8 @@
  */
 
 #include <ctype.h>
+#include <limits.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
@@ -104,6 +106,7 @@ kstat_cmp(const struct kstat_entry *ea, 
 RBT_PROTOTYPE(kstat_tree, kstat_entry, entry, kstat_cmp);
 RBT_GENERATE(kstat_tree, kstat_entry, entry, kstat_cmp);
 
+static void handle_alrm(int);
 static struct kstat_filter *
                kstat_filter_parse(char *);
 static int     kstat_filter_entry(struct kstat_filters *,
@@ -134,16 +137,17 @@ main(int argc, char *argv[])
        int fd;
        const char *errstr;
        int ch;
-       struct timespec interval = { 0, 0 };
+       struct itimerval itv;
+       sigset_t empty, mask;
        int i;
+       unsigned int wait = 0;
 
        while ((ch = getopt(argc, argv, "w:")) != -1) {
                switch (ch) {
                case 'w':
-                       interval.tv_sec = strtonum(optarg, 1, 100000000,
-                           &errstr);
+                       wait = strtonum(optarg, 1, UINT_MAX, &errstr);
                        if (errstr != NULL)
-                               errx(1, "wait %s: %s", optarg, errstr);
+                               errx(1, "wait is %s: %s", errstr, optarg);
                        break;
                default:
                        usage();
@@ -168,12 +172,25 @@ main(int argc, char *argv[])
        kstat_list(&kt, fd, version, &kfs);
        kstat_print(&kt);
 
-       if (interval.tv_sec == 0)
+       if (wait == 0)
                return (0);
 
-       for (;;) {
-               nanosleep(&interval, NULL);
+       if (signal(SIGALRM, handle_alrm) == SIG_ERR)
+               err(1, "signal");
+       sigemptyset(&empty);
+       sigemptyset(&mask);
+       sigaddset(&mask, SIGALRM);
+       if (sigprocmask(SIG_BLOCK, &mask, NULL) == -1)
+               err(1, "sigprocmask");
+
+       itv.it_value.tv_sec = wait;
+       itv.it_value.tv_usec = 0;
+       itv.it_interval = itv.it_value;
+       if (setitimer(ITIMER_REAL, &itv, NULL) == -1)
+               err(1, "setitimer");
 
+       for (;;) {
+               sigsuspend(&empty);
                kstat_read(&kt, fd);
                kstat_print(&kt);
        }
@@ -547,4 +564,9 @@ kstat_read(struct kstat_tree *kt, int fd
                if (ioctl(fd, KSTATIOC_FIND_ID, ksreq) == -1)
                        err(1, "update id %llu", ksreq->ks_id);
        }
+}
+
+static void
+handle_alrm(int signo)
+{
 }

Reply via email to