[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