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)
+{
}