From 3f5f3afa813fd2d2c06def4f6979deb45b9455d2 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Fri, 9 Mar 2018 11:54:40 +0200 Subject: [PATCH] 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 Reviewed-by: Derek Foreman Acked-by: Daniel Stone --- clients/clickdot.c | 36 +++-------- clients/window.c | 150 ++++++++++++++++++++++++++++++--------------- clients/window.h | 27 ++++++++ 3 files changed, 135 insertions(+), 78 deletions(-) diff --git a/clients/clickdot.c b/clients/clickdot.c index f52fbf04..f9e6e640 100644 --- a/clients/clickdot.c +++ b/clients/clickdot.c @@ -32,9 +32,8 @@ #include #include #include -#include -#include #include +#include #include #include @@ -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 15a86e15..7c3d3bdd 100644 --- 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 1ec9eac5..3366ab15 100644 --- a/clients/window.h +++ b/clients/window.h @@ -27,6 +27,7 @@ #include "config.h" #include +#include #include #include #include @@ -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