[PATCH v2 libinput 2/4] Move opening and closing the device fd into evdev.c

Peter Hutterer peter.hutterer at who-t.net
Wed Feb 5 15:20:15 PST 2014


evdev_device_remove() already calls close(device->fd). Move the
close_restricted call there to avoid one privileged call in the backend and
one in the device. And move the open_restricted() into the evdev device too to
reduce the duplicated code in the two backends.

Update to one of the tests: since we'd now fail getting the device node from
the invalid /tmp path, the open_func_count is 0.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
Changes to v1:
- move open_restricted into evdev_device_create()

If we really get an fd open failure, we now get two error messages, but I'll
fix that up in a follow-up restructure.

 src/evdev.c     | 18 +++++++++++++++---
 src/evdev.h     |  3 +--
 src/path.c      | 14 +-------------
 src/udev-seat.c | 20 +-------------------
 test/path.c     |  2 +-
 5 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/src/evdev.c b/src/evdev.c
index 2bc301b..61ab083 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -600,12 +600,22 @@ evdev_configure_device(struct evdev_device *device)
 struct evdev_device *
 evdev_device_create(struct libinput_seat *seat,
 		    const char *devnode,
-		    const char *sysname,
-		    int fd)
+		    const char *sysname)
 {
 	struct libinput *libinput = seat->libinput;
 	struct evdev_device *device;
 	char devname[256] = "unknown";
+	int fd;
+
+	/* Use non-blocking mode so that we can loop on read on
+	 * evdev_device_data() until all events on the fd are
+	 * read.  mtdev_get() also expects this. */
+	fd = open_restricted(libinput, devnode, O_RDWR | O_NONBLOCK);
+	if (fd < 0) {
+		log_info("opening input device '%s' failed (%s).\n",
+			 devnode, strerror(-fd));
+		return NULL;
+	}
 
 	device = zalloc(sizeof *device);
 	if (device == NULL)
@@ -655,6 +665,8 @@ evdev_device_create(struct libinput_seat *seat,
 	return device;
 
 err:
+	if (fd >= 0)
+		close_restricted(libinput, fd);
 	evdev_device_destroy(device);
 	return NULL;
 }
@@ -710,7 +722,7 @@ evdev_device_remove(struct evdev_device *device)
 
 	if (device->mtdev)
 		mtdev_close_delete(device->mtdev);
-	close(device->fd);
+	close_restricted(device->base.seat->libinput, device->fd);
 	list_remove(&device->base.link);
 
 	notify_removed_device(&device->base);
diff --git a/src/evdev.h b/src/evdev.h
index 37c32e5..3c9f93a 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -118,8 +118,7 @@ struct evdev_dispatch {
 struct evdev_device *
 evdev_device_create(struct libinput_seat *seat,
 		    const char *devnode,
-		    const char *sysname,
-		    int fd);
+		    const char *sysname);
 
 struct evdev_dispatch *
 evdev_touchpad_create(struct evdev_device *device);
diff --git a/src/path.c b/src/path.c
index 2893ad4..2254bbe 100644
--- a/src/path.c
+++ b/src/path.c
@@ -42,7 +42,6 @@ path_input_disable(struct libinput *libinput)
 	struct evdev_device *device = input->device;
 
 	if (device) {
-		close_restricted(libinput, device->fd);
 		evdev_device_remove(device);
 		input->device = NULL;
 	}
@@ -122,22 +121,13 @@ path_input_enable(struct libinput *libinput)
 	struct evdev_device *device;
 	const char *devnode = input->path;
 	char *sysname;
-	int fd;
 	char *seat_name, *seat_logical_name;
 
 	if (input->device)
 		return 0;
 
-	fd = open_restricted(libinput, devnode, O_RDWR|O_NONBLOCK);
-	if (fd < 0) {
-		log_info("opening input device '%s' failed (%s).\n",
-			 devnode, strerror(-fd));
-		return -1;
-	}
-
 	if (path_get_udev_properties(devnode, &sysname,
 				     &seat_name, &seat_logical_name) == -1) {
-		close_restricted(libinput, fd);
 		log_info("failed to obtain sysname for device '%s'.\n", devnode);
 		return -1;
 	}
@@ -146,16 +136,14 @@ path_input_enable(struct libinput *libinput)
 	free(seat_name);
 	free(seat_logical_name);
 
-	device = evdev_device_create(&seat->base, devnode, sysname, fd);
+	device = evdev_device_create(&seat->base, devnode, sysname);
 	free(sysname);
 	libinput_seat_unref(&seat->base);
 
 	if (device == EVDEV_UNHANDLED_DEVICE) {
-		close_restricted(libinput, fd);
 		log_info("not using input device '%s'.\n", devnode);
 		return -1;
 	} else if (device == NULL) {
-		close_restricted(libinput, fd);
 		log_info("failed to create input device '%s'.\n", devnode);
 		return -1;
 	}
diff --git a/src/udev-seat.c b/src/udev-seat.c
index 5936511..957e762 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -44,13 +44,11 @@ udev_seat_get_named(struct udev_input *input, const char *seat_name);
 static int
 device_added(struct udev_device *udev_device, struct udev_input *input)
 {
-	struct libinput *libinput = &input->base;
 	struct evdev_device *device;
 	const char *devnode;
 	const char *sysname;
 	const char *device_seat, *seat_name, *output_name;
 	const char *calibration_values;
-	int fd;
 	struct udev_seat *seat;
 
 	device_seat = udev_device_get_property_value(udev_device, "ID_SEAT");
@@ -78,26 +76,13 @@ device_added(struct udev_device *udev_device, struct udev_input *input)
 			return -1;
 	}
 
-	/* Use non-blocking mode so that we can loop on read on
-	 * evdev_device_data() until all events on the fd are
-	 * read.  mtdev_get() also expects this. */
-	fd = open_restricted(libinput, devnode, O_RDWR | O_NONBLOCK);
-	if (fd < 0) {
-		log_info("opening input device '%s' failed (%s).\n",
-			 devnode, strerror(-fd));
-		libinput_seat_unref(&seat->base);
-		return 0;
-	}
-
-	device = evdev_device_create(&seat->base, devnode, sysname, fd);
+	device = evdev_device_create(&seat->base, devnode, sysname);
 	libinput_seat_unref(&seat->base);
 
 	if (device == EVDEV_UNHANDLED_DEVICE) {
-		close_restricted(libinput, fd);
 		log_info("not using input device '%s'.\n", devnode);
 		return 0;
 	} else if (device == NULL) {
-		close_restricted(libinput, fd);
 		log_info("failed to create input device '%s'.\n", devnode);
 		return 0;
 	}
@@ -169,7 +154,6 @@ static void
 evdev_udev_handler(void *data)
 {
 	struct udev_input *input = data;
-	struct libinput *libinput = &input->base;
 	struct udev_device *udev_device;
 	struct evdev_device *device, *next;
 	const char *action;
@@ -198,7 +182,6 @@ evdev_udev_handler(void *data)
 				if (!strcmp(device->devnode, devnode)) {
 					log_info("input device %s, %s removed\n",
 						 device->devname, device->devnode);
-					close_restricted(libinput, device->fd);
 					evdev_device_remove(device);
 					break;
 				}
@@ -219,7 +202,6 @@ udev_input_remove_devices(struct udev_input *input)
 		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)) {
 				/* if the seat may be referenced by the
diff --git a/test/path.c b/test/path.c
index ec5d03d..c272e3a 100644
--- a/test/path.c
+++ b/test/path.c
@@ -89,7 +89,7 @@ START_TEST(path_create_invalid)
 	li = libinput_create_from_path(&simple_interface, NULL, path);
 	ck_assert(li == NULL);
 
-	ck_assert_int_eq(open_func_count, 1);
+	ck_assert_int_eq(open_func_count, 0);
 	ck_assert_int_eq(close_func_count, 0);
 
 	libinput_destroy(li);
-- 
1.8.4.2



More information about the wayland-devel mailing list