NetBSD support#36
Conversation
Make kqueue usage in status_bar less spammy.
| run_eventloop(sigset_t *signals) | ||
| { | ||
| struct pollfd fds[2]; | ||
| struct screen *screen; |
There was a problem hiding this comment.
Instead of using a separate event loop for kqueue, I think we should just avoid signalfd by using the self-pipe trick:
https://cr.yp.to/docs/selfpipe.html
There was a problem hiding this comment.
I've updated this to use the self-pipe trick but I'm not seeing the signal being fired more than once. Not sure what I'm doing wrong.
There was a problem hiding this comment.
It's working for me on Linux, so I'm not really sure.
If you're not getting the signal, it sounds like it has something to do with the timer. Unfortunately, I don't have any ideas about what might be wrong.
michaelforney
left a comment
michaelforney
left a comment
There was a problem hiding this comment.
Thanks, this looks pretty good. Just a few stylistic comments.
| struct screen *screen; | ||
| struct sigaction sa = {0}; | ||
|
|
||
| if (pipe(pfd) == -1) |
There was a problem hiding this comment.
Does NetBSD have pipe2? If so, let's just use that to create the pipe with O_NONBLOCK from the start.
It is accepted for the next version of POSIX:
https://www.austingroupbugs.net/view.php?id=411
| die("could not set O_NONBLOCK on pipe fd\n", strerror(errno)); | ||
|
|
||
| sa.sa_flags = SA_RESTART; | ||
| sa.sa_handler = sigalrm_handler; |
There was a problem hiding this comment.
I'd just move these to the initializer.
| if (poll(fds, sizeof(fds) / sizeof(fds[0]), -1) == -1) | ||
| break; | ||
| if (poll(fds, 2, -1) == -1) { | ||
| if (errno != EINTR) { |
There was a problem hiding this comment.
Let's leave sizeof(fds) / sizeof(fds[0]) in case the number of fds changes in the future.
Also, I'd combine these two if-statements:
if (poll(fds, sizeof(fds) / sizeof(fds[0]), -1) == -1 && errno != EINTR) {
perror("poll");
break;
}| struct tm *local_time; | ||
| int nbytes; | ||
|
|
||
| while ((nbytes = read(pfd[0], &discard, 1)) > 0); |
There was a problem hiding this comment.
No reason to save the return value nbytes if it's unused.
Also, let's move the semicolon to its own line to make it clear that the loop has an empty body.
| sigaddset(&signals, SIGALRM); | ||
| sigprocmask(SIG_BLOCK, &signals, NULL); | ||
| if (sigaction(SIGALRM, &sa, NULL) == -1) | ||
| die("sigaction failed\n", strerror(errno)); |
There was a problem hiding this comment.
This error format string is missing a %s.
Let's just use die("sigaction: %s\n", strerror(errno)).
| else ifeq ($(shell uname),FreeBSD) | ||
| FINAL_CPPFLAGS += -DHAVE_LINUX_INPUT_H | ||
| FINAL_CPPFLAGS += -DHAVE_LIBINPUT | ||
| FINAL_CPPFLAGS += -DHAVE_KQUEUE |
There was a problem hiding this comment.
HAVE_KQUEUE should not longer be needed with the self-pipe, right?
2 participants
linux/input.hor libinput.signalfdfor status_bar by adding an alternative kqueue event loop... I'm not entirely happy with this solution, honestly, but it seemed like the best solution to mimic the exact behaviour without adding an additional dependency on an event notification abstraction library. If it's not waiting on a signal the panel spins forever, which isn't great for power consumption.