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>

diff --git a/clients/clickdot.c b/clients/clickdot.c
index f52fbf0..f9e6e64 100644 (file)
--- a/clients/clickdot.c
+++ b/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 task cursor_timeout_task;
+       struct toytimer cursor_timeout;
 };
 
 static void
@@ -224,14 +222,7 @@ button_handler(struct widget *widget,
 static void
 cursor_timeout_reset(struct clickdot *clickdot)
 {
-       const long cursor_timeout = 500;
-       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);
+       toytimer_arm_once_usec(&clickdot->cursor_timeout, 500 * 1000);
 }
 
 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);
-       uint64_t exp;
-
-       if (read(clickdot->cursor_timeout_fd, &exp, sizeof (uint64_t)) !=
-           sizeof(uint64_t))
-               abort();
+               container_of(tt, struct clickdot, cursor_timeout);
 
        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 =
-               timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
-       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);
+       toytimer_init(&clickdot->cursor_timeout, CLOCK_MONOTONIC,
+                     display, cursor_timeout_func);
 
        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),
-                          clickdot->cursor_timeout_fd);
-       close(clickdot->cursor_timeout_fd);
+       toytimer_fini(&clickdot->cursor_timeout);
        if (clickdot->buffer)
                cairo_surface_destroy(clickdot->buffer);
        widget_destroy(clickdot->widget);

diff --git a/clients/window.c b/clients/window.c
index 15a86e1..7c3d3bd 100644 (file)
--- a/clients/window.c
+++ b/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;
-       int tooltip_fd;
+       struct toytimer timer;
        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 =
-               container_of(task, struct tooltip, tooltip_task);
-       uint64_t exp;
+       struct tooltip *tooltip = container_of(tt, struct tooltip, timer);
 
-       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;
-
-       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;
-       }
+       toytimer_arm_once_usec(&tooltip->timer, TOOLTIP_TIMEOUT * 1000);
 
        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);
-       if (tooltip->tooltip_fd < 0) {
-               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);
+       toytimer_init(&tooltip->timer, CLOCK_MONOTONIC,
+                     parent->window->display, tooltip_func);
        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;
-       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");
+       toytimer_arm_once_usec(&input->cursor_timer, duration * 1000);
 }
 
 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,
-                                               TFD_CLOEXEC | TFD_NONBLOCK);
-       display_watch_fd(d, input->cursor_delay_fd, EPOLLIN,
-                        &input->cursor_task);
+       toytimer_init(&input->cursor_timer, CLOCK_MONOTONIC, d,
+                     cursor_timer_func);
        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);
+}

diff --git a/clients/window.h b/clients/window.h
index 1ec9eac..3366ab1 100644 (file)
--- a/clients/window.h
+++ b/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