Fix leaking device groups

If a caller has a reference to a device group when the context is destroyed,
the memory for the group is never released. Calling
libinput_device_group_unref() will release it and there are no side-effects
since the group has no back-references. It's inconsistent with the rest of
libinput though - all other resources get released on libinput_unref().

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>

diff --git a/src/evdev.c b/src/evdev.c
index 094320eb0bc8d597f6f99489fec429f4c2c9359e..080e0a1240358cea4b3aedfda86c3c4db5dabbfb 100644 (file)
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -2084,27 +2084,17 @@ static int
 evdev_set_device_group(struct evdev_device *device,
                       struct udev_device *udev_device)
 {
+       struct libinput *libinput = device->base.seat->libinput;
        struct libinput_device_group *group = NULL;
        const char *udev_group;
 
        udev_group = udev_device_get_property_value(udev_device,
                                                    "LIBINPUT_DEVICE_GROUP");
-       if (udev_group) {
-               struct libinput_device *d;
-
-               list_for_each(d, &device->base.seat->devices_list, link) {
-                       const char *identifier = d->group->identifier;
-
-                       if (identifier &&
-                           strcmp(identifier, udev_group) == 0) {
-                               group = d->group;
-                               break;
-                       }
-               }
-       }
+       if (udev_group)
+               group = libinput_device_group_find_group(libinput, udev_group);
 
        if (!group) {
-               group = libinput_device_group_create(udev_group);
+               group = libinput_device_group_create(libinput, udev_group);
                if (!group)
                        return 1;
                libinput_device_set_device_group(&device->base, group);

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 8b161cc347882abb52df8a649d184fdecc4ea084..be99af5e735771a61280c9da5592ae5d6b85e12f 100644 (file)
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -91,6 +91,8 @@ struct libinput {
        enum libinput_log_priority log_priority;
        void *user_data;
        int refcount;
+
+       struct list device_group_list;
 };
 
 typedef void (*libinput_seat_destroy_func) (struct libinput_seat *seat);
@@ -224,6 +226,8 @@ struct libinput_device_group {
        int refcount;
        void *user_data;
        char *identifier; /* unique identifier or NULL for singletons */
+
+       struct list link;
 };
 
 struct libinput_device {
@@ -321,7 +325,12 @@ libinput_device_init(struct libinput_device *device,
                     struct libinput_seat *seat);
 
 struct libinput_device_group *
-libinput_device_group_create(const char *identifier);
+libinput_device_group_create(struct libinput *libinput,
+                            const char *identifier);
+
+struct libinput_device_group *
+libinput_device_group_find_group(struct libinput *libinput,
+                                const char *identifier);
 
 void
 libinput_device_set_device_group(struct libinput_device *device,

diff --git a/src/libinput.c b/src/libinput.c
index e5645714e7779111173ec1fe488a3bc855b288e8..b88bab11f00210311d58757173785faaafe323d2 100644 (file)
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -211,6 +211,9 @@ libinput_log_set_handler(struct libinput *libinput,
        libinput->log_handler = log_handler;
 }
 
+static void
+libinput_device_group_destroy(struct libinput_device_group *group);
+
 static void
 libinput_post_event(struct libinput *libinput,
                    struct libinput_event *event);
@@ -944,6 +947,7 @@ libinput_init(struct libinput *libinput,
        libinput->refcount = 1;
        list_init(&libinput->source_destroy_list);
        list_init(&libinput->seat_list);
+       list_init(&libinput->device_group_list);
 
        if (libinput_timer_subsys_init(libinput) != 0) {
                free(libinput->events);
@@ -983,6 +987,7 @@ libinput_unref(struct libinput *libinput)
        struct libinput_event *event;
        struct libinput_device *device, *next_device;
        struct libinput_seat *seat, *next_seat;
+       struct libinput_device_group *group, *next_group;
 
        if (libinput == NULL)
                return NULL;
@@ -1010,6 +1015,13 @@ libinput_unref(struct libinput *libinput)
                libinput_seat_destroy(seat);
        }
 
+       list_for_each_safe(group,
+                          next_group,
+                          &libinput->device_group_list,
+                          link) {
+               libinput_device_group_destroy(group);
+       }
+
        libinput_timer_subsys_destroy(libinput);
        libinput_drop_destroyed_sources(libinput);
        close(libinput->epoll_fd);
@@ -1962,7 +1974,8 @@ libinput_device_group_ref(struct libinput_device_group *group)
 }
 
 struct libinput_device_group *
-libinput_device_group_create(const char *identifier)
+libinput_device_group_create(struct libinput *libinput,
+                            const char *identifier)
 {
        struct libinput_device_group *group;
 
@@ -1975,13 +1988,32 @@ libinput_device_group_create(const char *identifier)
                group->identifier = strdup(identifier);
                if (!group->identifier) {
                        free(group);
-                       group = NULL;
+                       return NULL;
                }
        }
 
+       list_init(&group->link);
+       list_insert(&libinput->device_group_list, &group->link);
+
        return group;
 }
 
+struct libinput_device_group *
+libinput_device_group_find_group(struct libinput *libinput,
+                                const char *identifier)
+{
+       struct libinput_device_group *g = NULL;
+
+       list_for_each(g, &libinput->device_group_list, link) {
+               if (identifier && g->identifier &&
+                   streq(g->identifier, identifier)) {
+                       return g;
+               }
+       }
+
+       return g;
+}
+
 void
 libinput_device_set_device_group(struct libinput_device *device,
                                 struct libinput_device_group *group)
@@ -1993,6 +2025,7 @@ libinput_device_set_device_group(struct libinput_device *device,
 static void
 libinput_device_group_destroy(struct libinput_device_group *group)
 {
+       list_remove(&group->link);
        free(group->identifier);
        free(group);
 }

diff --git a/test/device.c b/test/device.c
index 2aa090c4f24df1be0297d79f4c2f949cf6b16175..b7fa0e07bff710eff810dfa864f3acf653eb77f3 100644 (file)
--- a/test/device.c
+++ b/test/device.c
@@ -674,6 +674,22 @@ START_TEST(device_context)
 }
 END_TEST
 
+static int open_restricted(const char *path, int flags, void *data)
+{
+       int fd;
+       fd = open(path, flags);
+       return fd < 0 ? -errno : fd;
+}
+static void close_restricted(int fd, void *data)
+{
+       close(fd);
+}
+
+const struct libinput_interface simple_interface = {
+       .open_restricted = open_restricted,
+       .close_restricted = close_restricted,
+};
+
 START_TEST(device_group_get)
 {
        struct litest_device *dev = litest_current_device();
@@ -722,6 +738,36 @@ START_TEST(device_group_ref)
 }
 END_TEST
 
+START_TEST(device_group_leak)
+{
+       struct libinput *li;
+       struct libinput_device *device;
+       struct libevdev_uinput *uinput;
+       struct libinput_device_group *group;
+
+       uinput = litest_create_uinput_device("test device", NULL,
+                                            EV_KEY, BTN_LEFT,
+                                            EV_KEY, BTN_RIGHT,
+                                            EV_REL, REL_X,
+                                            EV_REL, REL_Y,
+                                            -1);
+
+       li = libinput_path_create_context(&simple_interface, NULL);
+       device = libinput_path_add_device(li,
+                                         libevdev_uinput_get_devnode(uinput));
+
+       group = libinput_device_get_device_group(device);
+       libinput_device_group_ref(group);
+
+       libinput_path_remove_device(device);
+
+       libevdev_uinput_destroy(uinput);
+       libinput_unref(li);
+
+       /* the device group leaks, check valgrind */
+}
+END_TEST
+
 START_TEST(abs_device_no_absx)
 {
        struct libevdev_uinput *uinput;
@@ -1239,6 +1285,7 @@ litest_setup_tests(void)
 
        litest_add("device:group", device_group_get, LITEST_ANY, LITEST_ANY);
        litest_add_no_device("device:group", device_group_ref);
+       litest_add_no_device("device:group", device_group_leak);
 
        litest_add_no_device("device:invalid devices", abs_device_no_absx);
        litest_add_no_device("device:invalid devices", abs_device_no_absy);