Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ TARGETS := velox.pc velox
SUBDIRS := protocol clients
CLEAN_FILES := $(TARGETS)

VELOX_PACKAGES = swc xkbcommon libinput
VELOX_PACKAGES = swc xkbcommon
VELOX_SOURCES = \
config.c \
layout.c \
Expand All @@ -31,6 +31,10 @@ VELOX_SOURCES = \
window.c \
protocol/velox-protocol.c

ifneq ($(shell uname),NetBSD)
VELOX_PACKAGES += libinput
endif

ifeq ($(if $(V),$(V),0), 0)
define quiet
@echo " $1 $@"
Expand Down Expand Up @@ -62,6 +66,18 @@ else
FINAL_CPPFLAGS += -DNDEBUG
endif

ifeq ($(shell uname),Linux)
FINAL_CPPFLAGS += -DHAVE_LINUX_INPUT_H
FINAL_CPPFLAGS += -DHAVE_LIBINPUT
else ifeq ($(shell uname),FreeBSD)
FINAL_CPPFLAGS += -DHAVE_LINUX_INPUT_H
FINAL_CPPFLAGS += -DHAVE_LIBINPUT
FINAL_CPPFLAGS += -DHAVE_KQUEUE

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAVE_KQUEUE should not longer be needed with the self-pipe, right?

else ifeq ($(shell uname),NetBSD)
FINAL_CPPFLAGS += -D_NETBSD_SOURCE
FINAL_CPPFLAGS += -DHAVE_KQUEUE
endif

compile = $(call quiet,CC) $(FINAL_CPPFLAGS) $(FINAL_CFLAGS) -c -o $@ $< \
-MMD -MP -MF .deps/$(basename $<).d -MT $(basename $@).o
link = $(call quiet,CCLD,$(CC)) $(LDFLAGS) -o $@ $^
Expand Down
73 changes: 60 additions & 13 deletions clients/status_bar.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@
#include <stdlib.h>
#include <string.h>
#include <poll.h>
#include <sys/signalfd.h>
#include <time.h>
#include <fcntl.h>
#include <unistd.h>
#include <wayland-client.h>
#include <wld/wayland.h>
#include <wld/wld.h>
Expand Down Expand Up @@ -172,6 +173,7 @@ static bool running, need_draw;
static char clock_text[32];
static struct item_data divider_data = {.width = 14 };
static struct text_item_data clock_data = {.text = clock_text };
static int pfd[2];

static void __attribute__((noreturn)) die(const char *const format, ...)
{
Expand Down Expand Up @@ -504,32 +506,74 @@ setup(void)
wl_display_flush(display);
}

static void
sigalrm_handler(int sig)
{
int p_errno = errno; /* Preserve errno */
int ret;

if ((ret = write(pfd[1], ".", 1)) != 1)
die("self-pipe write failed\n");
errno = p_errno;
}

static int
set_nonblock(int fd)
{
int flags;

if ((flags = fcntl(fd, F_GETFL)) == -1) {
return -1;
}

if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) {
return -1;
}
return 0;
}


static void
run(void)
{
sigset_t signals;
struct itimerspec timer_value = {
.it_interval = { 1, 0 },
.it_value = { 0, 1 }
};
struct pollfd fds[2];
struct screen *screen;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@alarixnia alarixnia Mar 6, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

struct sigaction sa = {0};

if (pipe(pfd) == -1)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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("creating pipe failed: %s\n", strerror(errno));

if (set_nonblock(pfd[0]) == -1)
die("could not set O_NONBLOCK on pipe fd: %s\n", strerror(errno));

if (set_nonblock(pfd[1]) == -1)
die("could not set O_NONBLOCK on pipe fd\n", strerror(errno));

sa.sa_flags = SA_RESTART;
sa.sa_handler = sigalrm_handler;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just move these to the initializer.


sigemptyset(&signals);
sigaddset(&signals, SIGALRM);
sigprocmask(SIG_BLOCK, &signals, NULL);
if (sigaction(SIGALRM, &sa, NULL) == -1)
die("sigaction failed\n", strerror(errno));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error format string is missing a %s.

Let's just use die("sigaction: %s\n", strerror(errno)).


fds[0].fd = wl_display_get_fd(display);
fds[0].events = POLLIN;
fds[1].fd = signalfd(-1, &signals, SFD_CLOEXEC);
fds[1].fd = pfd[0];
fds[1].events = POLLIN;

timer_settime(timer, 0, &timer_value, NULL);
running = true;

while (true) {
if (poll(fds, sizeof(fds) / sizeof(fds[0]), -1) == -1)
break;
if (poll(fds, 2, -1) == -1) {
if (errno != EINTR) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

perror("poll");
break;
}
}

if (fds[0].revents & POLLIN) {
if (wl_display_dispatch(display) == -1) {
Expand All @@ -539,11 +583,14 @@ run(void)
}
}
if (fds[1].revents & POLLIN) {
time_t raw_time = time(NULL);
struct tm *local_time = localtime(&raw_time);

sigwaitinfo(&signals, NULL);

uint8_t discard;
time_t raw_time;
struct tm *local_time;
int nbytes;

while ((nbytes = read(pfd[0], &discard, 1)) > 0);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

raw_time = time(NULL);
local_time = localtime(&raw_time);
strftime(clock_text, sizeof(clock_text), "%A %T %F", local_time);
update_text_item_data(&clock_data);
}
Expand Down
8 changes: 8 additions & 0 deletions config.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@
#include "velox.h"

#include <fcntl.h>
#ifdef HAVE_LINUX_INPUT_H
#include <linux/input.h>
#else
#define BTN_LEFT (0x110)
#define BTN_RIGHT (0x111)
#define BTN_MIDDLE (0x112)
#define BTN_SIDE (0x113)
#define BTN_EXTRA (0x114)
#endif
#include <spawn.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down
4 changes: 4 additions & 0 deletions velox.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
#include "window.h"
#include "protocol/velox-server-protocol.h"

#ifdef HAVE_LIBINPUT
#include <libinput.h>
#endif
#include <limits.h>
#include <signal.h>
#include <spawn.h>
Expand Down Expand Up @@ -72,8 +74,10 @@ new_window(struct swc_window *swc)
static void
new_device(struct libinput_device *device)
{
#ifdef HAVE_LIBINPUT
libinput_device_config_tap_set_enabled(device, tap_to_click ?
LIBINPUT_CONFIG_TAP_ENABLED : LIBINPUT_CONFIG_TAP_DISABLED);
#endif
}

const struct swc_manager manager = {
Expand Down