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

Jonas Ådahl jadahl at gmail.com
Thu Feb 6 13:15:25 PST 2014


On Thu, Feb 06, 2014 at 09:20:15AM +1000, Peter Hutterer wrote:
> 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.

This good I think.

Jonas

> 
> 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