Drop seat events

seats are more a compositor concept than a concept of the input library. All
devices in a libinput context are associated with the seat given on creation
of the seat (maps to ID_SEAT in udev for the udev backend).

A logical seat may be assigned to a device (e.g. WL_SEAT) but this does not
necessarily map to the creation of the seat in the compositor.
Drop the seat events but keep seat objects around so that the caller can still
identify which seat a device belongs to.

If the libinput_seat_unref() in the udev backend destroys the seat, the device
list of that seat is invalid and we'd be accessing already freed bytes. To
avoid this, ref the seat before the device removal loop, then unref it once
we're done - that unref then may trigger the actual removal of the seat.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 3414388..b08c035 100644 (file)
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -107,12 +107,6 @@ libinput_device_init(struct libinput_device *device,
                     struct libinput_seat *seat);
 
 void
-notify_added_seat(struct libinput_seat *seat);
-
-void
-notify_removed_seat(struct libinput_seat *seat);
-
-void
 notify_added_device(struct libinput_device *device);
 
 void

diff --git a/src/libinput.c b/src/libinput.c
index d2e7596..61e7102 100644 (file)
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -54,16 +54,6 @@ struct libinput_event {
        union libinput_event_target target;
 };
 
-struct libinput_event_added_seat {
-       struct libinput_event base;
-       struct libinput_seat *seat;
-};
-
-struct libinput_event_removed_seat {
-       struct libinput_event base;
-       struct libinput_seat *seat;
-};
-
 struct libinput_event_added_device {
        struct libinput_event base;
        struct libinput_device *device;
@@ -140,18 +130,6 @@ libinput_event_get_context(struct libinput_event *event)
        return event->libinput;
 }
 
-LIBINPUT_EXPORT struct libinput_seat *
-libinput_event_added_seat_get_seat(struct libinput_event_added_seat *event)
-{
-       return event->seat;
-}
-
-LIBINPUT_EXPORT struct libinput_seat *
-libinput_event_removed_seat_get_seat(struct libinput_event_removed_seat *event)
-{
-       return event->seat;
-}
-
 LIBINPUT_EXPORT struct libinput_device *
 libinput_event_added_device_get_device(
        struct libinput_event_added_device *event)
@@ -429,8 +407,6 @@ libinput_event_get_class(struct libinput_event *event)
        case LIBINPUT_EVENT_NONE:
                return LIBINPUT_EVENT_CLASS_NONE;
 
-       case LIBINPUT_EVENT_ADDED_SEAT:
-       case LIBINPUT_EVENT_REMOVED_SEAT:
        case LIBINPUT_EVENT_ADDED_DEVICE:
        case LIBINPUT_EVENT_REMOVED_DEVICE:
                return LIBINPUT_EVENT_CLASS_BASE;
@@ -466,10 +442,6 @@ libinput_event_destroy(struct libinput_event *event)
                break;
        }
 
-       if (libinput_event_get_type(event) == LIBINPUT_EVENT_ADDED_SEAT ||
-           libinput_event_get_type(event) == LIBINPUT_EVENT_REMOVED_SEAT)
-               libinput_seat_unref(((struct libinput_event_added_seat*)event)->seat);
-
        free(event);
 }
 
@@ -633,46 +605,6 @@ post_device_event(struct libinput_device *device,
 }
 
 void
-notify_added_seat(struct libinput_seat *seat)
-{
-       struct libinput_event_added_seat *added_seat_event;
-
-       added_seat_event = malloc(sizeof *added_seat_event);
-       if (!added_seat_event)
-               return;
-
-       libinput_seat_ref(seat);
-
-       *added_seat_event = (struct libinput_event_added_seat) {
-               .seat = seat,
-       };
-
-       post_base_event(seat->libinput,
-                       LIBINPUT_EVENT_ADDED_SEAT,
-                       &added_seat_event->base);
-}
-
-void
-notify_removed_seat(struct libinput_seat *seat)
-{
-       struct libinput_event_removed_seat *removed_seat_event;
-
-       removed_seat_event = malloc(sizeof *removed_seat_event);
-       if (!removed_seat_event)
-               return;
-
-       libinput_seat_ref(seat);
-
-       *removed_seat_event = (struct libinput_event_removed_seat) {
-               .seat = seat,
-       };
-
-       post_base_event(seat->libinput,
-                       LIBINPUT_EVENT_REMOVED_SEAT,
-                       &removed_seat_event->base);
-}
-
-void
 notify_added_device(struct libinput_device *device)
 {
        struct libinput_event_added_device *added_device_event;

diff --git a/src/libinput.h b/src/libinput.h
index 5f4f51e..52c7c1a 100644 (file)
--- a/src/libinput.h
+++ b/src/libinput.h
@@ -124,8 +124,6 @@ enum libinput_touch_type {
  */
 enum libinput_event_type {
        LIBINPUT_EVENT_NONE = 0,
-       LIBINPUT_EVENT_ADDED_SEAT,
-       LIBINPUT_EVENT_REMOVED_SEAT,
        LIBINPUT_EVENT_ADDED_DEVICE,
        LIBINPUT_EVENT_REMOVED_DEVICE,
 
@@ -150,8 +148,6 @@ union libinput_event_target {
 };
 
 struct libinput_event;
-struct libinput_event_added_seat;
-struct libinput_event_removed_seat;
 struct libinput_event_added_device;
 struct libinput_event_removed_device;
 struct libinput_event_keyboard_key;
@@ -252,20 +248,6 @@ struct libinput*
 libinput_event_get_context(struct libinput_event *event);
 
 /**
- * @defgroup event_added_seat Added seat event
- */
-
-struct libinput_seat *
-libinput_event_added_seat_get_seat(struct libinput_event_added_seat *event);
-
-/**
- * @defgroup event_removed_seat Removed seat event
- */
-
-struct libinput_seat *
-libinput_event_removed_seat_get_seat(struct libinput_event_removed_seat *event);
-
-/**
  * @defgroup event_added_device Added device event
  */
 

diff --git a/src/path.c b/src/path.c
index cafe1fe..10f6b1e 100644 (file)
--- a/src/path.c
+++ b/src/path.c
@@ -47,7 +47,6 @@ path_input_disable(struct libinput *libinput)
 
        /* should only be one seat anyway */
        list_for_each_safe(seat, tmp, &libinput->seat_list, base.link) {
-               notify_removed_seat(&seat->base);
                list_remove(&seat->base.link);
                list_init(&seat->base.link);
                libinput_seat_unref(&seat->base);
@@ -74,7 +73,6 @@ path_seat_create(struct path_input *input)
 
        libinput_seat_init(&seat->base, &input->base, seat->name, path_seat_destroy);
        list_insert(&input->base.seat_list, &seat->base.link);
-       notify_added_seat(&seat->base);
 
        return seat;
 }

diff --git a/src/udev-seat.c b/src/udev-seat.c
index 610b45d..1522c30 100644 (file)
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -198,8 +198,6 @@ evdev_udev_handler(void *data)
                                                 device->devname, device->devnode);
                                        close_restricted(libinput, device->fd);
                                        evdev_device_remove(device);
-                                       if (list_empty(&seat->base.devices_list))
-                                               notify_removed_seat(&seat->base);
                                        libinput_seat_unref(&seat->base);
                                        break;
                                }
@@ -217,21 +215,22 @@ udev_input_remove_devices(struct udev_input *input)
        struct udev_seat *seat, *tmp;
 
        list_for_each_safe(seat, tmp, &input->base.seat_list, base.link) {
+               libinput_seat_ref(&seat->base);
                list_for_each_safe(device, next,
                                   &seat->base.devices_list, base.link) {
                        close_restricted(&input->base, device->fd);
                        evdev_device_remove(device);
                        if (list_empty(&seat->base.devices_list)) {
-                               notify_removed_seat(&seat->base);
-                               /* seat is referenced by the event, so make
-                                  sure it's dropped from the seat list now,
-                                  to be freed whenever the device is
-                                  removed */
+                               /* if the seat may be referenced by the
+                                  client, so make sure it's dropped from
+                                  the seat list now, to be freed whenever
+                                  the device is removed */
                                list_remove(&seat->base.link);
                                list_init(&seat->base.link);
                        }
                        libinput_seat_unref(&seat->base);
                }
+               libinput_seat_unref(&seat->base);
        }
 }
 
@@ -327,7 +326,6 @@ udev_seat_create(struct udev_input *input, const char *seat_name)
 
        libinput_seat_init(&seat->base, &input->base, seat_name, udev_seat_destroy);
        list_insert(&input->base.seat_list, &seat->base.link);
-       notify_added_seat(&seat->base);
 
        return seat;
 }

diff --git a/test/path.c b/test/path.c
index 27f7c04..6a9da75 100644 (file)
--- a/test/path.c
+++ b/test/path.c
@@ -134,7 +134,8 @@ START_TEST(path_added_seat)
        struct litest_device *dev = litest_current_device();
        struct libinput *li = dev->libinput;
        struct libinput_event *event;
-       struct libinput_event_added_seat *seat_event;
+       struct libinput_event_added_device *device_event;
+       struct libinput_device *device;
        struct libinput_seat *seat;
        const char *seat_name;
        enum libinput_event_type type;
@@ -145,10 +146,11 @@ START_TEST(path_added_seat)
        ck_assert(event != NULL);
 
        type = libinput_event_get_type(event);
-       ck_assert_int_eq(type, LIBINPUT_EVENT_ADDED_SEAT);
+       ck_assert_int_eq(type, LIBINPUT_EVENT_ADDED_DEVICE);
 
-       seat_event = (struct libinput_event_added_seat*)event;
-       seat = libinput_event_added_seat_get_seat(seat_event);
+       device_event = (struct libinput_event_added_device*)event;
+       device = libinput_event_added_device_get_device(device_event);
+       seat = libinput_device_get_seat(device);
        ck_assert(seat != NULL);
 
        seat_name = libinput_seat_get_name(seat);

diff --git a/test/udev.c b/test/udev.c
index f40533a..50f0521 100644 (file)
--- a/test/udev.c
+++ b/test/udev.c
@@ -138,7 +138,8 @@ START_TEST(udev_added_seat_default)
        struct libinput *li;
        struct libinput_event *event;
        struct udev *udev;
-       struct libinput_event_added_seat *seat_event;
+       struct libinput_event_added_device *device_event;
+       struct libinput_device *device;
        struct libinput_seat *seat;
        const char *seat_name;
        enum libinput_event_type type;
@@ -153,13 +154,14 @@ START_TEST(udev_added_seat_default)
 
        while (!default_seat_found && (event = libinput_get_event(li))) {
                type = libinput_event_get_type(event);
-               if (type != LIBINPUT_EVENT_ADDED_SEAT) {
+               if (type != LIBINPUT_EVENT_ADDED_DEVICE) {
                        libinput_event_destroy(event);
                        continue;
                }
 
-               seat_event = (struct libinput_event_added_seat*)event;
-               seat = libinput_event_added_seat_get_seat(seat_event);
+               device_event = (struct libinput_event_added_device*)event;
+               device = libinput_event_added_device_get_device(device_event);
+               seat = libinput_device_get_seat(device);
                ck_assert(seat != NULL);
 
                seat_name = libinput_seat_get_name(seat);
@@ -174,93 +176,6 @@ START_TEST(udev_added_seat_default)
 }
 END_TEST
 
-START_TEST(udev_removed_seat)
-{
-       struct libinput *li;
-       struct libinput_event *event;
-       struct udev *udev;
-#define MAX_SEATS 30
-       char *seat_names[MAX_SEATS] = {0};
-       int idx = 0;
-
-       udev = udev_new();
-       ck_assert(udev != NULL);
-
-       li = libinput_create_from_udev(&simple_interface, NULL, udev, "seat0");
-       ck_assert(li != NULL);
-       libinput_dispatch(li);
-
-       while ((event = libinput_get_event(li))) {
-               enum libinput_event_type type;
-               struct libinput_event_added_seat *seat_event;
-               struct libinput_seat *seat;
-               const char *seat_name;
-
-               type = libinput_event_get_type(event);
-               if (type != LIBINPUT_EVENT_ADDED_SEAT) {
-                       libinput_event_destroy(event);
-                       continue;
-               }
-
-               seat_event = (struct libinput_event_added_seat*)event;
-               seat = libinput_event_added_seat_get_seat(seat_event);
-               ck_assert(seat != NULL);
-
-               seat_name = libinput_seat_get_name(seat);
-               seat_names[idx++] = strdup(seat_name);
-               ck_assert_int_lt(idx, MAX_SEATS);
-
-               libinput_event_destroy(event);
-       }
-
-       libinput_suspend(li);
-
-       while ((event = libinput_get_event(li))) {
-               enum libinput_event_type type;
-               struct libinput_event_removed_seat *seat_event;
-               struct libinput_seat *seat;
-               const char *seat_name;
-               int i;
-
-               type = libinput_event_get_type(event);
-               if (type != LIBINPUT_EVENT_REMOVED_SEAT) {
-                       libinput_event_destroy(event);
-                       continue;
-               }
-
-               seat_event = (struct libinput_event_removed_seat*)event;
-               seat = libinput_event_removed_seat_get_seat(seat_event);
-               ck_assert(seat != NULL);
-
-               seat_name = libinput_seat_get_name(seat);
-
-               for (i = 0; i < idx; i++) {
-                       if (seat_names[i] &&
-                           strcmp(seat_names[i], seat_name) == 0) {
-                               free(seat_names[i]);
-                               seat_names[i] = NULL;
-                               break;
-                       }
-               }
-               ck_assert_msg(i < idx,
-                             "Seat '%s' unknown or already removed\n",
-                             seat_name);
-
-               libinput_event_destroy(event);
-       }
-
-       while(idx--) {
-               ck_assert_msg(seat_names[idx] == NULL,
-                             "Seat '%s' not removed\n",
-                             seat_names[idx]);
-       }
-
-       libinput_destroy(li);
-       udev_unref(udev);
-#undef MAX_SEATS
-}
-END_TEST
-
 START_TEST(udev_double_suspend)
 {
        struct libinput *li;
@@ -388,7 +303,6 @@ int main (int argc, char **argv) {
        litest_add_no_device("udev:create", udev_create_empty_seat);
 
        litest_add_no_device("udev:seat events", udev_added_seat_default);
-       litest_add_no_device("udev:seat events", udev_removed_seat);
 
        litest_add("udev:suspend", udev_double_suspend, LITEST_ANY, LITEST_ANY);
        litest_add("udev:suspend", udev_double_resume, LITEST_ANY, LITEST_ANY);