From 57c36ddcb346e1e2443ad606ff214dc4a74b97b4 Mon Sep 17 00:00:00 2001 From: emersion Date: Sat, 28 Apr 2018 12:38:03 +0100 Subject: [PATCH 1/3] backend/wayland: add proper error handling to wlr_wl_backend_create --- backend/wayland/backend.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/backend/wayland/backend.c b/backend/wayland/backend.c index 8ad5ba15..54c814d9 100644 --- a/backend/wayland/backend.c +++ b/backend/wayland/backend.c @@ -176,7 +176,8 @@ static void handle_display_destroy(struct wl_listener *listener, void *data) { backend_destroy(&backend->backend); } -struct wlr_backend *wlr_wl_backend_create(struct wl_display *display, const char *remote) { +struct wlr_backend *wlr_wl_backend_create(struct wl_display *display, + const char *remote) { wlr_log(L_INFO, "Creating wayland backend"); struct wlr_wl_backend *backend = calloc(1, sizeof(struct wlr_wl_backend)); @@ -194,26 +195,40 @@ struct wlr_backend *wlr_wl_backend_create(struct wl_display *display, const char backend->remote_display = wl_display_connect(remote); if (!backend->remote_display) { wlr_log_errno(L_ERROR, "Could not connect to remote display"); - return false; + goto error_connect; } backend->registry = wl_display_get_registry(backend->remote_display); if (backend->registry == NULL) { wlr_log_errno(L_ERROR, "Could not obtain reference to remote registry"); - return false; + goto error_registry; } - wlr_egl_init(&backend->egl, EGL_PLATFORM_WAYLAND_EXT, - backend->remote_display, NULL, WL_SHM_FORMAT_ARGB8888); + if (!wlr_egl_init(&backend->egl, EGL_PLATFORM_WAYLAND_EXT, + backend->remote_display, NULL, WL_SHM_FORMAT_ARGB8888)) { + wlr_log(L_ERROR, "Could not initialize EGL"); + goto error_egl; + } wlr_egl_bind_display(&backend->egl, backend->local_display); backend->renderer = wlr_gles2_renderer_create(&backend->egl); if (backend->renderer == NULL) { - wlr_log_errno(L_ERROR, "Could not create renderer"); + wlr_log(L_ERROR, "Could not create renderer"); + goto error_renderer; } backend->local_display_destroy.notify = handle_display_destroy; wl_display_add_destroy_listener(display, &backend->local_display_destroy); return &backend->backend; + +error_renderer: + wlr_egl_finish(&backend->egl); +error_egl: + wl_registry_destroy(backend->registry); +error_registry: + wl_display_disconnect(backend->remote_display); +error_connect: + free(backend); + return NULL; } From 79da4c175eca6f2db5e167a9e3c40ef343d6cd87 Mon Sep 17 00:00:00 2001 From: emersion Date: Sat, 28 Apr 2018 12:47:28 +0100 Subject: [PATCH 2/3] backend/headless: remove useless destructor --- backend/headless/input_device.c | 10 +--------- backend/libinput/keyboard.c | 4 +++- backend/x11/backend.c | 11 +++-------- types/wlr_keyboard.c | 8 ++++---- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/backend/headless/input_device.c b/backend/headless/input_device.c index daa22436..a1e18428 100644 --- a/backend/headless/input_device.c +++ b/backend/headless/input_device.c @@ -9,15 +9,7 @@ #include "backend/headless.h" #include "util/signal.h" -static void input_device_destroy(struct wlr_input_device *wlr_dev) { - struct wlr_headless_input_device *device = - (struct wlr_headless_input_device *)wlr_dev; - free(device); -} - -static const struct wlr_input_device_impl input_device_impl = { - .destroy = input_device_destroy, -}; +static const struct wlr_input_device_impl input_device_impl = { 0 }; bool wlr_input_device_is_headless(struct wlr_input_device *wlr_dev) { return wlr_dev->impl == &input_device_impl; diff --git a/backend/libinput/keyboard.c b/backend/libinput/keyboard.c index d8dd8878..e17191e3 100644 --- a/backend/libinput/keyboard.c +++ b/backend/libinput/keyboard.c @@ -13,7 +13,8 @@ struct wlr_libinput_keyboard { }; static void keyboard_set_leds(struct wlr_keyboard *wlr_kb, uint32_t leds) { - struct wlr_libinput_keyboard *wlr_libinput_kb = (struct wlr_libinput_keyboard *)wlr_kb; + struct wlr_libinput_keyboard *wlr_libinput_kb = + (struct wlr_libinput_keyboard *)wlr_kb; libinput_device_led_update(wlr_libinput_kb->libinput_dev, leds); } @@ -21,6 +22,7 @@ static void keyboard_destroy(struct wlr_keyboard *wlr_kb) { struct wlr_libinput_keyboard *wlr_libinput_kb = (struct wlr_libinput_keyboard *)wlr_kb; libinput_device_unref(wlr_libinput_kb->libinput_dev); + free(wlr_libinput_kb); } struct wlr_keyboard_impl impl = { diff --git a/backend/x11/backend.c b/backend/x11/backend.c index 35d037b0..f64515ca 100644 --- a/backend/x11/backend.c +++ b/backend/x11/backend.c @@ -223,14 +223,9 @@ static void backend_destroy(struct wlr_backend *backend) { wlr_signal_emit_safe(&x11->pointer_dev.events.destroy, &x11->pointer_dev); wlr_signal_emit_safe(&x11->keyboard_dev.events.destroy, &x11->keyboard_dev); - // TODO probably need to use wlr_keyboard_destroy, but the devices need to - // be malloced for that to work - if (x11->keyboard_dev.keyboard->keymap) { - xkb_keymap_unref(x11->keyboard_dev.keyboard->keymap); - } - if (x11->keyboard_dev.keyboard->xkb_state) { - xkb_state_unref(x11->keyboard_dev.keyboard->xkb_state); - } + + wlr_input_device_destroy(&x11->keyboard_dev); + wlr_input_device_destroy(&x11->pointer_dev); wlr_signal_emit_safe(&backend->events.destroy, backend); diff --git a/types/wlr_keyboard.c b/types/wlr_keyboard.c index 8a5bd7e2..1d173d7a 100644 --- a/types/wlr_keyboard.c +++ b/types/wlr_keyboard.c @@ -154,15 +154,15 @@ void wlr_keyboard_destroy(struct wlr_keyboard *kb) { if (kb == NULL) { return; } + xkb_state_unref(kb->xkb_state); + xkb_keymap_unref(kb->keymap); + close(kb->keymap_fd); if (kb->impl && kb->impl->destroy) { kb->impl->destroy(kb); } else { wl_list_remove(&kb->events.key.listener_list); + free(kb); } - xkb_state_unref(kb->xkb_state); - xkb_keymap_unref(kb->keymap); - close(kb->keymap_fd); - free(kb); } void wlr_keyboard_led_update(struct wlr_keyboard *kb, uint32_t leds) { From f8e0a034512d4c5a69dde4f5cd02df53af216b72 Mon Sep 17 00:00:00 2001 From: emersion Date: Sat, 28 Apr 2018 12:55:36 +0100 Subject: [PATCH 3/3] backend/x11: correctly destroy input devices --- backend/x11/backend.c | 4 ++-- backend/x11/input_device.c | 24 +++++++++++++++++++++++- include/backend/x11.h | 2 ++ include/wlr/interfaces/wlr_keyboard.h | 3 ++- include/wlr/interfaces/wlr_pointer.h | 2 +- include/wlr/types/wlr_keyboard.h | 3 +-- include/wlr/types/wlr_pointer.h | 2 +- types/wlr_keyboard.c | 2 +- types/wlr_pointer.c | 2 +- 9 files changed, 34 insertions(+), 10 deletions(-) diff --git a/backend/x11/backend.c b/backend/x11/backend.c index f64515ca..f14bbbe6 100644 --- a/backend/x11/backend.c +++ b/backend/x11/backend.c @@ -317,12 +317,12 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display, wlr_input_device_init(&x11->keyboard_dev, WLR_INPUT_DEVICE_KEYBOARD, &input_device_impl, "X11 keyboard", 0, 0); - wlr_keyboard_init(&x11->keyboard, NULL); + wlr_keyboard_init(&x11->keyboard, &keyboard_impl); x11->keyboard_dev.keyboard = &x11->keyboard; wlr_input_device_init(&x11->pointer_dev, WLR_INPUT_DEVICE_POINTER, &input_device_impl, "X11 pointer", 0, 0); - wlr_pointer_init(&x11->pointer, NULL); + wlr_pointer_init(&x11->pointer, &pointer_impl); x11->pointer_dev.pointer = &x11->pointer; x11->display_destroy.notify = handle_display_destroy; diff --git a/backend/x11/input_device.c b/backend/x11/input_device.c index 5de5b4c2..42b703cf 100644 --- a/backend/x11/input_device.c +++ b/backend/x11/input_device.c @@ -136,7 +136,29 @@ void handle_x11_input_event(struct wlr_x11_backend *x11, } } -const struct wlr_input_device_impl input_device_impl = { 0 }; +static void input_device_destroy(struct wlr_input_device *wlr_device) { + // Don't free the input device, it's on the stack +} + +const struct wlr_input_device_impl input_device_impl = { + .destroy = input_device_destroy, +}; + +static void keyboard_destroy(struct wlr_keyboard *wlr_keyboard) { + // Don't free the keyboard, it's on the stack +} + +const struct wlr_keyboard_impl keyboard_impl = { + .destroy = keyboard_destroy, +}; + +static void pointer_destroy(struct wlr_pointer *wlr_pointer) { + // Don't free the pointer, it's on the stack +} + +const struct wlr_pointer_impl pointer_impl = { + .destroy = pointer_destroy, +}; void update_x11_pointer_position(struct wlr_x11_output *output, xcb_timestamp_t time) { diff --git a/include/backend/x11.h b/include/backend/x11.h index 6ab1cec6..1d56fbe8 100644 --- a/include/backend/x11.h +++ b/include/backend/x11.h @@ -77,6 +77,8 @@ struct wlr_x11_output *get_x11_output_from_window_id(struct wlr_x11_backend *x11 void get_x11_output_layout_box(struct wlr_x11_backend *backend, struct wlr_box *box); +extern const struct wlr_keyboard_impl keyboard_impl; +extern const struct wlr_pointer_impl pointer_impl; extern const struct wlr_input_device_impl input_device_impl; void handle_x11_input_event(struct wlr_x11_backend *x11, diff --git a/include/wlr/interfaces/wlr_keyboard.h b/include/wlr/interfaces/wlr_keyboard.h index 6960ea87..c9a13fd7 100644 --- a/include/wlr/interfaces/wlr_keyboard.h +++ b/include/wlr/interfaces/wlr_keyboard.h @@ -9,7 +9,8 @@ struct wlr_keyboard_impl { void (*led_update)(struct wlr_keyboard *keyboard, uint32_t leds); }; -void wlr_keyboard_init(struct wlr_keyboard *keyboard, struct wlr_keyboard_impl *impl); +void wlr_keyboard_init(struct wlr_keyboard *keyboard, + const struct wlr_keyboard_impl *impl); void wlr_keyboard_destroy(struct wlr_keyboard *keyboard); void wlr_keyboard_notify_key(struct wlr_keyboard *keyboard, struct wlr_event_keyboard_key *event); diff --git a/include/wlr/interfaces/wlr_pointer.h b/include/wlr/interfaces/wlr_pointer.h index af677b97..f0cf9081 100644 --- a/include/wlr/interfaces/wlr_pointer.h +++ b/include/wlr/interfaces/wlr_pointer.h @@ -8,7 +8,7 @@ struct wlr_pointer_impl { }; void wlr_pointer_init(struct wlr_pointer *pointer, - struct wlr_pointer_impl *impl); + const struct wlr_pointer_impl *impl); void wlr_pointer_destroy(struct wlr_pointer *pointer); #endif diff --git a/include/wlr/types/wlr_keyboard.h b/include/wlr/types/wlr_keyboard.h index ed0427e8..bfb4e611 100644 --- a/include/wlr/types/wlr_keyboard.h +++ b/include/wlr/types/wlr_keyboard.h @@ -40,8 +40,7 @@ struct wlr_keyboard_modifiers { }; struct wlr_keyboard { - struct wlr_keyboard_impl *impl; - // TODO: Should this store key repeat info too? + const struct wlr_keyboard_impl *impl; int keymap_fd; size_t keymap_size; diff --git a/include/wlr/types/wlr_pointer.h b/include/wlr/types/wlr_pointer.h index a8969b9e..45619e0a 100644 --- a/include/wlr/types/wlr_pointer.h +++ b/include/wlr/types/wlr_pointer.h @@ -8,7 +8,7 @@ struct wlr_pointer_impl; struct wlr_pointer { - struct wlr_pointer_impl *impl; + const struct wlr_pointer_impl *impl; struct { struct wl_signal motion; diff --git a/types/wlr_keyboard.c b/types/wlr_keyboard.c index 1d173d7a..0f0cb1cf 100644 --- a/types/wlr_keyboard.c +++ b/types/wlr_keyboard.c @@ -138,7 +138,7 @@ void wlr_keyboard_notify_key(struct wlr_keyboard *keyboard, } void wlr_keyboard_init(struct wlr_keyboard *kb, - struct wlr_keyboard_impl *impl) { + const struct wlr_keyboard_impl *impl) { kb->impl = impl; wl_signal_init(&kb->events.key); wl_signal_init(&kb->events.modifiers); diff --git a/types/wlr_pointer.c b/types/wlr_pointer.c index 9d5dc08c..20e75731 100644 --- a/types/wlr_pointer.c +++ b/types/wlr_pointer.c @@ -5,7 +5,7 @@ #include void wlr_pointer_init(struct wlr_pointer *pointer, - struct wlr_pointer_impl *impl) { + const struct wlr_pointer_impl *impl) { pointer->impl = impl; wl_signal_init(&pointer->events.motion); wl_signal_init(&pointer->events.motion_absolute);