clients: consolidate timer code part 1

There are multiple copies for the timerfd handling code, and I need a
timer in one more app. Consolidate all the timerfd code into window.c to
reduce the duplication. Many of the copies were also flawed against the
race mentioned in toytimer_fire().

This patch handles clickdot and window.c's tooltip timer and cursor
timer.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>

clients/clickdot.c

@@ -32,9 +32,8 @@
 #include <cairo.h>
 #include <math.h>
 #include <assert.h>
-#include <sys/timerfd.h>
-#include <sys/epoll.h>
 #include <unistd.h>
+#include <time.h>
 
 #include <linux/input.h>
 #include <wayland-client.h>
@@ -62,8 +61,7 @@ struct clickdot {
 	int reset;
 
 	struct input *cursor_timeout_input;
-	int cursor_timeout_fd;
+	struct toytimer cursor_timeout;
-	struct task cursor_timeout_task;
 };
 
 static void
@@ -224,14 +222,7 @@ button_handler(struct widget *widget,
 static void
 cursor_timeout_reset(struct clickdot *clickdot)
 {
-	const long cursor_timeout = 500;
+	toytimer_arm_once_usec(&clickdot->cursor_timeout, 500 * 1000);
-	struct itimerspec its;
-
-	its.it_interval.tv_sec = 0;
-	its.it_interval.tv_nsec = 0;
-	its.it_value.tv_sec = cursor_timeout / 1000;
-	its.it_value.tv_nsec = (cursor_timeout % 1000) * 1000 * 1000;
-	timerfd_settime(clickdot->cursor_timeout_fd, 0, &its, NULL);
 }
 
 static int
@@ -271,15 +262,10 @@ leave_handler(struct widget *widget,
 }
 
 static void
-cursor_timeout_func(struct task *task, uint32_t events)
+cursor_timeout_func(struct toytimer *tt)
 {
 	struct clickdot *clickdot =
-		container_of(task, struct clickdot, cursor_timeout_task);
+		container_of(tt, struct clickdot, cursor_timeout);
-	uint64_t exp;
-
-	if (read(clickdot->cursor_timeout_fd, &exp, sizeof (uint64_t)) !=
-	    sizeof(uint64_t))
-		abort();
 
 	input_set_pointer_image(clickdot->cursor_timeout_input,
 				CURSOR_LEFT_PTR);
@@ -317,12 +303,8 @@ clickdot_create(struct display *display)
 	clickdot->line.old_y = -1;
 	clickdot->reset = 0;
 
-	clickdot->cursor_timeout_fd =
+	toytimer_init(&clickdot->cursor_timeout, CLOCK_MONOTONIC,
-		timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
+		      display, cursor_timeout_func);
-	clickdot->cursor_timeout_task.run = cursor_timeout_func;
-	display_watch_fd(window_get_display(clickdot->window),
-			 clickdot->cursor_timeout_fd,
-			 EPOLLIN, &clickdot->cursor_timeout_task);
 
 	return clickdot;
 }
@@ -330,9 +312,7 @@ clickdot_create(struct display *display)
 static void
 clickdot_destroy(struct clickdot *clickdot)
 {
-	display_unwatch_fd(window_get_display(clickdot->window),
+	toytimer_fini(&clickdot->cursor_timeout);
-			   clickdot->cursor_timeout_fd);
-	close(clickdot->cursor_timeout_fd);
 	if (clickdot->buffer)
 		cairo_surface_destroy(clickdot->buffer);
 	widget_destroy(clickdot->widget);

clients/window.c

@@ -349,9 +349,8 @@ struct input {
 	struct wl_callback *cursor_frame_cb;
 	uint32_t cursor_timer_start;
 	uint32_t cursor_anim_current;
-	int cursor_delay_fd;
+	struct toytimer cursor_timer;
 	bool cursor_timer_running;
-	struct task cursor_task;
 	struct wl_surface *pointer_surface;
 	uint32_t modifiers;
 	uint32_t pointer_enter_serial;
@@ -440,8 +439,7 @@ struct tooltip {
 	struct widget *parent;
 	struct widget *widget;
 	char *entry;
-	struct task tooltip_task;
+	struct toytimer timer;
-	int tooltip_fd;
 	float x, y;
 };
 
@@ -2122,21 +2120,17 @@ widget_destroy_tooltip(struct widget *parent)
 		tooltip->widget = NULL;
 	}
 
-	close(tooltip->tooltip_fd);
+	toytimer_fini(&tooltip->timer);
 	free(tooltip->entry);
 	free(tooltip);
 	parent->tooltip = NULL;
 }
 
 static void
-tooltip_func(struct task *task, uint32_t events)
+tooltip_func(struct toytimer *tt)
 {
-	struct tooltip *tooltip =
+	struct tooltip *tooltip = container_of(tt, struct tooltip, timer);
-		container_of(task, struct tooltip, tooltip_task);
-	uint64_t exp;
 
-	if (read(tooltip->tooltip_fd, &exp, sizeof (uint64_t)) != sizeof (uint64_t))
-		abort();
 	window_create_tooltip(tooltip);
 }
 
@@ -2144,16 +2138,7 @@ tooltip_func(struct task *task, uint32_t events)
 static int
 tooltip_timer_reset(struct tooltip *tooltip)
 {
-	struct itimerspec its;
+	toytimer_arm_once_usec(&tooltip->timer, TOOLTIP_TIMEOUT * 1000);
-
-	its.it_interval.tv_sec = 0;
-	its.it_interval.tv_nsec = 0;
-	its.it_value.tv_sec = TOOLTIP_TIMEOUT / 1000;
-	its.it_value.tv_nsec = (TOOLTIP_TIMEOUT % 1000) * 1000 * 1000;
-	if (timerfd_settime(tooltip->tooltip_fd, 0, &its, NULL) < 0) {
-		fprintf(stderr, "could not set timerfd\n: %m");
-		return -1;
-	}
 
 	return 0;
 }
@@ -2186,15 +2171,8 @@ widget_set_tooltip(struct widget *parent, char *entry, float x, float y)
 	tooltip->x = x;
 	tooltip->y = y;
 	tooltip->entry = strdup(entry);
-	tooltip->tooltip_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
+	toytimer_init(&tooltip->timer, CLOCK_MONOTONIC,
-	if (tooltip->tooltip_fd < 0) {
+		      parent->window->display, tooltip_func);
-		fprintf(stderr, "could not create timerfd\n: %m");
-		return -1;
-	}
-
-	tooltip->tooltip_task.run = tooltip_func;
-	display_watch_fd(parent->window->display, tooltip->tooltip_fd,
-			 EPOLLIN, &tooltip->tooltip_task);
 	tooltip_timer_reset(tooltip);
 
 	return 0;
@@ -2685,19 +2663,12 @@ input_ungrab(struct input *input)
 static void
 cursor_delay_timer_reset(struct input *input, uint32_t duration)
 {
-	struct itimerspec its;
-
 	if (!duration)
 		input->cursor_timer_running = false;
 	else
 		input->cursor_timer_running = true;
 
-	its.it_interval.tv_sec = 0;
+	toytimer_arm_once_usec(&input->cursor_timer, duration * 1000);
-	its.it_interval.tv_nsec = 0;
-	its.it_value.tv_sec = duration / 1000;
-	its.it_value.tv_nsec = (duration % 1000) * 1000 * 1000;
-	if (timerfd_settime(input->cursor_delay_fd, 0, &its, NULL) < 0)
-		fprintf(stderr, "could not set cursor timerfd\n: %m");
 }
 
 static void cancel_pointer_image_update(struct input *input)
@@ -3888,20 +3859,16 @@ pointer_surface_frame_callback(void *data, struct wl_callback *callback,
 }
 
 static void
-cursor_timer_func(struct task *task, uint32_t events)
+cursor_timer_func(struct toytimer *tt)
 {
-	struct input *input = container_of(task, struct input, cursor_task);
+	struct input *input = container_of(tt, struct input, cursor_timer);
 	struct timespec tp;
 	struct wl_cursor *cursor;
 	uint32_t time;
-	uint64_t exp;
 
 	if (!input->cursor_timer_running)
 		return;
 
-	if (read(input->cursor_delay_fd, &exp, sizeof (uint64_t)) != sizeof (uint64_t))
-		return;
-
 	cursor = input->display->cursors[input->current_cursor];
 	if (!cursor)
 		return;
@@ -5876,12 +5843,9 @@ display_add_input(struct display *d, uint32_t id, int display_seat_version)
 	}
 
 	input->pointer_surface = wl_compositor_create_surface(d->compositor);
-	input->cursor_task.run = cursor_timer_func;
 
-	input->cursor_delay_fd = timerfd_create(CLOCK_MONOTONIC,
+	toytimer_init(&input->cursor_timer, CLOCK_MONOTONIC, d,
-						TFD_CLOEXEC | TFD_NONBLOCK);
+		      cursor_timer_func);
-	display_watch_fd(d, input->cursor_delay_fd, EPOLLIN,
-			 &input->cursor_task);
 	set_repeat_info(input, 40, 400);
 
 	input->repeat_timer_fd = timerfd_create(CLOCK_MONOTONIC,
@@ -5932,7 +5896,7 @@ input_destroy(struct input *input)
 	wl_list_remove(&input->link);
 	wl_seat_destroy(input->seat);
 	close(input->repeat_timer_fd);
-	close(input->cursor_delay_fd);
+	toytimer_fini(&input->cursor_timer);
 	free(input);
 }
 
@@ -6562,3 +6526,89 @@ keysym_modifiers_get_mask(struct wl_array *modifiers_map,
 
 	return 1 << index;
 }
+
+static void
+toytimer_fire(struct task *tsk, uint32_t events)
+{
+	uint64_t e;
+	struct toytimer *tt;
+
+	tt = container_of(tsk, struct toytimer, tsk);
+
+	if (events != EPOLLIN)
+		fprintf(stderr, "unexpected timerfd events %x\n", events);
+
+	if (!(events & EPOLLIN))
+		return;
+
+	if (read(tt->fd, &e, sizeof e) != sizeof e) {
+		/* If we change the timer between the fd becoming
+		 * readable and getting here, there'll be nothing to
+		 * read and we get EAGAIN. */
+		if (errno != EAGAIN)
+			fprintf(stderr, "timer read failed: %m\n");
+		return;
+	}
+
+	tt->callback(tt);
+}
+
+void
+toytimer_init(struct toytimer *tt, clockid_t clock, struct display *display,
+	      toytimer_cb callback)
+{
+	memset(tt, 0, sizeof *tt);
+
+	tt->fd = timerfd_create(clock, TFD_CLOEXEC | TFD_NONBLOCK);
+	if (tt->fd == -1) {
+		fprintf(stderr, "creating timer failed: %m\n");
+		abort();
+	}
+
+	tt->display = display;
+	tt->callback = callback;
+	tt->tsk.run = toytimer_fire;
+	display_watch_fd(display, tt->fd, EPOLLIN, &tt->tsk);
+}
+
+void
+toytimer_fini(struct toytimer *tt)
+{
+	display_unwatch_fd(tt->display, tt->fd);
+	close(tt->fd);
+	tt->fd = -1;
+}
+
+void
+toytimer_arm(struct toytimer *tt, const struct itimerspec *its)
+{
+	int ret;
+
+	ret = timerfd_settime(tt->fd, 0, its, NULL);
+	if (ret < 0) {
+		fprintf(stderr, "timer setup failed: %m\n");
+		abort();
+	}
+}
+
+#define USEC_PER_SEC 1000000
+
+void
+toytimer_arm_once_usec(struct toytimer *tt, uint32_t usec)
+{
+	struct itimerspec its;
+
+	its.it_interval.tv_sec = 0;
+	its.it_interval.tv_nsec = 0;
+	its.it_value.tv_sec = usec / USEC_PER_SEC;
+	its.it_value.tv_nsec = (usec % USEC_PER_SEC) * 1000;
+	toytimer_arm(tt, &its);
+}
+
+void
+toytimer_disarm(struct toytimer *tt)
+{
+	struct itimerspec its = {};
+
+	toytimer_arm(tt, &its);
+}

clients/window.h

@@ -27,6 +27,7 @@
 #include "config.h"
 
 #include <stdint.h>
+#include <time.h>
 #include <xkbcommon/xkbcommon.h>
 #include <wayland-client.h>
 #include <cairo.h>
@@ -713,4 +714,30 @@ xkb_mod_mask_t
 keysym_modifiers_get_mask(struct wl_array *modifiers_map,
 			  const char *name);
 
+struct toytimer;
+typedef void (*toytimer_cb)(struct toytimer *);
+
+struct toytimer {
+	struct display *display;
+	struct task tsk;
+	int fd;
+	toytimer_cb callback;
+};
+
+void
+toytimer_init(struct toytimer *tt, clockid_t clock, struct display *display,
+	      toytimer_cb callback);
+
+void
+toytimer_fini(struct toytimer *tt);
+
+void
+toytimer_arm(struct toytimer *tt, const struct itimerspec *its);
+
+void
+toytimer_arm_once_usec(struct toytimer *tt, uint32_t usec);
+
+void
+toytimer_disarm(struct toytimer *tt);
+
 #endif