interface/wlr_tablet_pad: rework destroy sequence

The destroy callback in wlr_tablet_pad_impl has been removed. The function
`wlr_tablet_pad_finish` has been introduced to clean up the resources owned by a
wlr_tablet_pad.

`wlr_input_device_destroy` no longer destroys the wlr_tablet_pad, attempting to
destroy a wlr_tablet_pad will result in a no-op.

The field `name` has been added to the wlr_tablet_pad_impl to be able to identify
a given wlr_tablet_pad device.

backend/libinput/events.c

@@ -30,7 +30,7 @@ void destroy_libinput_input_device(struct wlr_libinput_input_device *dev) {
 		wlr_tablet_destroy(&dev->tablet);
 	}
 	if (dev->tablet_pad.impl) {
-		wlr_tablet_pad_destroy(&dev->tablet_pad);
+		finish_device_tablet_pad(dev);
 	}
 
 	libinput_device_unref(dev->handle);

backend/libinput/tablet_pad.c

@@ -8,6 +8,10 @@
 #include "backend/libinput.h"
 #include "util/signal.h"
 
+const struct wlr_tablet_pad_impl libinput_tablet_pad_impl = {
+	.name = "libinput-tablet-pad",
+};
+
 static void group_destroy(struct wlr_tablet_pad_group *group) {
 	free(group->buttons);
 	free(group->strips);
@@ -15,7 +19,6 @@ static void group_destroy(struct wlr_tablet_pad_group *group) {
 	free(group);
 }
 
-// FIXME: Decide on how to alloc/count here
 static void add_pad_group_from_libinput(struct wlr_tablet_pad *pad,
 		struct libinput_device *device, unsigned int index) {
 	struct libinput_tablet_pad_mode_group *li_group =
@@ -88,27 +91,6 @@ group_fail:
 	group_destroy(group);
 }
 
-static void tablet_pad_destroy(struct wlr_tablet_pad *wlr_tablet_pad) {
-	struct wlr_libinput_input_device *dev =
-		device_from_tablet_pad(wlr_tablet_pad);
-
-	struct wlr_tablet_pad_group *group, *tmp;
-	wl_list_for_each_safe(group, tmp, &wlr_tablet_pad->groups, link) {
-		group_destroy(group);
-	}
-
-	int groups = libinput_device_tablet_pad_get_num_mode_groups(dev->handle);
-	for (int i = 0; i < groups; ++i) {
-		struct libinput_tablet_pad_mode_group *li_group =
-			libinput_device_tablet_pad_get_mode_group(dev->handle, i);
-		libinput_tablet_pad_mode_group_unref(li_group);
-	}
-}
-
-const struct wlr_tablet_pad_impl libinput_tablet_pad_impl = {
-	.destroy = tablet_pad_destroy,
-};
-
 void init_device_tablet_pad(struct wlr_libinput_input_device *dev) {
 	struct libinput_device *handle = dev->handle;
 	const char *name = libinput_device_get_name(handle);
@@ -134,6 +116,22 @@ void init_device_tablet_pad(struct wlr_libinput_input_device *dev) {
 	}
 }
 
+void finish_device_tablet_pad(struct wlr_libinput_input_device *dev) {
+	struct wlr_tablet_pad_group *group, *tmp;
+	wl_list_for_each_safe(group, tmp, &dev->tablet_pad.groups, link) {
+		group_destroy(group);
+	}
+
+	wlr_tablet_pad_finish(&dev->tablet_pad);
+
+	int groups = libinput_device_tablet_pad_get_num_mode_groups(dev->handle);
+	for (int i = 0; i < groups; ++i) {
+		struct libinput_tablet_pad_mode_group *li_group =
+			libinput_device_tablet_pad_get_mode_group(dev->handle, i);
+		libinput_tablet_pad_mode_group_unref(li_group);
+	}
+}
+
 struct wlr_libinput_input_device *device_from_tablet_pad(
 		struct wlr_tablet_pad *wlr_tablet_pad) {
 	assert(wlr_tablet_pad->impl == &libinput_tablet_pad_impl);

backend/wayland/seat.c

@@ -559,6 +559,10 @@ void destroy_wl_input_device(struct wlr_wl_input_device *dev) {
 		case WLR_INPUT_DEVICE_SWITCH:
 			wlr_switch_finish(wlr_dev->switch_device);
 			break;
+		case WLR_INPUT_DEVICE_TABLET_PAD:
+			wlr_tablet_pad_finish(wlr_dev->tablet_pad);
+			free(wlr_dev->tablet_pad);
+			break;
 		default:
 			wlr_input_device_destroy(wlr_dev);
 			break;

backend/wayland/tablet_v2.c

@@ -289,10 +289,11 @@ static void handle_tablet_pad_group_removed(
 
 	zwp_tablet_pad_group_v2_destroy(group->pad_group);
 
-	/* While I'm pretty sure we have control over this as well, it's
+	free(group->group.buttons);
-	 * outside the scope of a single function, so better be safe than
+	free(group->group.strips);
-	 * sorry */
+	free(group->group.rings);
 	wl_list_remove(&group->group.link);
+
 	free(group);
 }
 
@@ -396,22 +397,15 @@ static void handle_tablet_pad_leave(void *data,
 static void handle_tablet_pad_removed(void *data,
 		struct zwp_tablet_pad_v2 *zwp_tablet_pad_v2) {
 	struct wlr_wl_input_device *dev = data;
-	struct wlr_tablet_pad *tablet_pad = dev->wlr_input_device.tablet_pad;
-
-	/* This doesn't free anything, but emits the destroy signal */
-	wlr_input_device_destroy(&dev->wlr_input_device);
-	/* This is a bit ugly, but we need to remove it from our list */
-	wl_list_remove(&dev->link);
 
+	struct wlr_tablet_pad *tablet_pad = dev->wlr_input_device.tablet_pad;
 	struct wlr_wl_tablet_pad_group *group, *it;
 	wl_list_for_each_safe(group, it, &tablet_pad->groups, group.link) {
 		handle_tablet_pad_group_removed(group);
 	}
 
-	/* This frees */
-	wlr_tablet_pad_destroy(tablet_pad);
 	zwp_tablet_pad_v2_destroy(dev->resource);
-	free(dev);
+	destroy_wl_input_device(dev);
 }
 
 static const struct zwp_tablet_pad_v2_listener tablet_pad_listener = {
@@ -425,7 +419,9 @@ static const struct zwp_tablet_pad_v2_listener tablet_pad_listener = {
 	.removed = handle_tablet_pad_removed,
 };
 
-const struct wlr_tablet_pad_impl tablet_pad_impl = {0};
+const struct wlr_tablet_pad_impl tablet_pad_impl = {
+	.name = "wl-tablet-pad",
+};
 
 static void handle_pad_added(void *data,
 		struct zwp_tablet_seat_v2 *zwp_tablet_seat_v2,
@@ -454,7 +450,7 @@ static void handle_pad_added(void *data,
 		zwp_tablet_pad_v2_destroy(id);
 		return;
 	}
-	wlr_tablet_pad_init(wlr_dev->tablet_pad, &tablet_pad_impl, wlr_dev->name);
+	wlr_tablet_pad_init(wlr_dev->tablet_pad, &tablet_pad_impl, "wlr_tablet_v2");
 	zwp_tablet_pad_v2_add_listener(id, &tablet_pad_listener, dev);
 }
 

include/backend/libinput.h

@@ -120,6 +120,7 @@ void handle_tablet_tool_button(struct libinput_event *event,
 	struct wlr_tablet *tablet);
 
 void init_device_tablet_pad(struct wlr_libinput_input_device *dev);
+void finish_device_tablet_pad(struct wlr_libinput_input_device *dev);
 struct wlr_libinput_input_device *device_from_tablet_pad(
 	struct wlr_tablet_pad *tablet_pad);
 void handle_tablet_pad_button(struct libinput_event *event,

include/wlr/interfaces/wlr_tablet_pad.h

@@ -12,11 +12,17 @@
 #include <wlr/types/wlr_tablet_pad.h>
 
 struct wlr_tablet_pad_impl {
-	void (*destroy)(struct wlr_tablet_pad *pad);
+	const char *name;
 };
 
 void wlr_tablet_pad_init(struct wlr_tablet_pad *pad,
 	const struct wlr_tablet_pad_impl *impl, const char *name);
-void wlr_tablet_pad_destroy(struct wlr_tablet_pad *pad);
+
+/**
+ * Cleans up the resources owned by a wlr_tablet_pad.
+ * This function will not clean the memory allocated by wlr_tablet_pad_group,
+ * it's the responsibility of the caller to clean it.
+ */
+void wlr_tablet_pad_finish(struct wlr_tablet_pad *pad);
 
 #endif

types/wlr_input_device.c

@@ -2,7 +2,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <wayland-server-core.h>
-#include <wlr/interfaces/wlr_tablet_pad.h>
 #include <wlr/interfaces/wlr_tablet_tool.h>
 #include <wlr/interfaces/wlr_touch.h>
 #include <wlr/types/wlr_input_device.h>
@@ -53,7 +52,7 @@ void wlr_input_device_destroy(struct wlr_input_device *dev) {
 			wlr_tablet_destroy(dev->tablet);
 			break;
 		case WLR_INPUT_DEVICE_TABLET_PAD:
-			wlr_tablet_pad_destroy(dev->tablet_pad);
+			wlr_log(WLR_ERROR, "wlr_tablet_pad will not be destroyed");
 			break;
 		}
 	} else {

types/wlr_tablet_pad.c

@@ -3,6 +3,7 @@
 #include <wayland-server-core.h>
 #include <wlr/interfaces/wlr_tablet_pad.h>
 #include <wlr/types/wlr_tablet_pad.h>
+#include <wlr/util/log.h>
 
 void wlr_tablet_pad_init(struct wlr_tablet_pad *pad,
 		const struct wlr_tablet_pad_impl *impl, const char *name) {
@@ -19,10 +20,8 @@ void wlr_tablet_pad_init(struct wlr_tablet_pad *pad,
 	wl_array_init(&pad->paths);
 }
 
-void wlr_tablet_pad_destroy(struct wlr_tablet_pad *pad) {
+void wlr_tablet_pad_finish(struct wlr_tablet_pad *pad) {
-	if (!pad) {
+	wlr_input_device_finish(&pad->base);
-		return;
-	}
 
 	char **path_ptr;
 	wl_array_for_each(path_ptr, &pad->paths) {
@@ -30,10 +29,8 @@ void wlr_tablet_pad_destroy(struct wlr_tablet_pad *pad) {
 	}
 	wl_array_release(&pad->paths);
 
-	wlr_input_device_finish(&pad->base);
+	/* TODO: wlr_tablet_pad should own its wlr_tablet_pad_group */
-	if (pad->impl && pad->impl->destroy) {
+	if (!wl_list_empty(&pad->groups)) {
-		pad->impl->destroy(pad);
+		wlr_log(WLR_ERROR, "wlr_tablet_pad groups is not empty");
-	} else {
-		free(pad);
 	}
 }