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 <[email protected]> > --- > 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 > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
