[PATCH libinput 4/9] evdev: use a udev_device instead of separate sysname/syspath/devnode
Hans de Goede
hdegoede at redhat.com
Mon Nov 24 00:55:45 PST 2014
Hi,
On 11/24/2014 01:46 AM, Peter Hutterer wrote:
> Using a udev_device instead of the various bits separately safes us
> re-initializing udev contexts whenever we need to compare the device. And
> having the actual udev device makes it a bit easier to ensure that we're not
> re-initializing a different device as a current one.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> src/evdev.c | 85 +++++++++++++++++++++++----------------------------------
> src/evdev.h | 9 ++----
> src/path.c | 11 ++------
> src/udev-seat.c | 16 +++++------
> 4 files changed, 46 insertions(+), 75 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index fed66cb..8d457f2 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -240,7 +240,8 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> if (device->mt.slots[slot].seat_slot != -1) {
> log_bug_kernel(libinput,
> "%s: Driver sent multiple touch down for the "
> - "same slot", device->devnode);
> + "same slot",
> + udev_device_get_devnode(device->udev_device));
> break;
> }
>
> @@ -292,7 +293,8 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> if (device->abs.seat_slot != -1) {
> log_bug_kernel(libinput,
> "%s: Driver sent multiple touch down for the "
> - "same slot", device->devnode);
> + "same slot",
> + udev_device_get_devnode(device->udev_device));
> break;
> }
>
> @@ -1218,20 +1220,11 @@ evdev_need_mtdev(struct evdev_device *device)
> static void
> evdev_tag_device(struct evdev_device *device)
> {
> - struct udev *udev;
> - struct udev_device *udev_device = NULL;
> + struct udev_device *udev_device;
>
> - udev = udev_new();
> - if (!udev)
> - return;
> -
> - udev_device = udev_device_new_from_syspath(udev, device->syspath);
> - if (udev_device) {
> - if (device->dispatch->interface->tag_device)
> - device->dispatch->interface->tag_device(device, udev_device);
> - udev_device_unref(udev_device);
> - }
> - udev_unref(udev);
> + udev_device = device->udev_device;
> + if (device->dispatch->interface->tag_device)
> + device->dispatch->interface->tag_device(device, udev_device);
Maybe reduce the function to just these 2 lines ? :
if (device->dispatch->interface->tag_device)
device->dispatch->interface->tag_device(device, device->udev_device);
Otherwise looks good:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> }
>
> static inline int
> @@ -1266,6 +1259,7 @@ evdev_configure_device(struct evdev_device *device)
> int active_slot;
> int slot;
> unsigned int i;
> + const char *devnode = udev_device_get_devnode(device->udev_device);
>
> has_rel = 0;
> has_abs = 0;
> @@ -1364,7 +1358,7 @@ evdev_configure_device(struct evdev_device *device)
> device->dispatch = evdev_mt_touchpad_create(device);
> log_info(libinput,
> "input device '%s', %s is a touchpad\n",
> - device->devname, device->devnode);
> + device->devname, devnode);
> return device->dispatch == NULL ? -1 : 0;
> }
>
> @@ -1397,7 +1391,7 @@ evdev_configure_device(struct evdev_device *device)
>
> log_info(libinput,
> "input device '%s', %s is a pointer caps =%s%s%s\n",
> - device->devname, device->devnode,
> + device->devname, devnode,
> has_abs ? " absolute-motion" : "",
> has_rel ? " relative-motion": "",
> has_button ? " button" : "");
> @@ -1417,13 +1411,13 @@ evdev_configure_device(struct evdev_device *device)
> device->seat_caps |= EVDEV_DEVICE_KEYBOARD;
> log_info(libinput,
> "input device '%s', %s is a keyboard\n",
> - device->devname, device->devnode);
> + device->devname, devnode);
> }
> if (has_touch && !has_button) {
> device->seat_caps |= EVDEV_DEVICE_TOUCH;
> log_info(libinput,
> "input device '%s', %s is a touch device\n",
> - device->devname, device->devnode);
> + device->devname, devnode);
> }
>
> return 0;
> @@ -1457,15 +1451,14 @@ evdev_notify_added_device(struct evdev_device *device)
>
> struct evdev_device *
> evdev_device_create(struct libinput_seat *seat,
> - const char *devnode,
> - const char *sysname,
> - const char *syspath)
> + struct udev_device *udev_device)
> {
> struct libinput *libinput = seat->libinput;
> struct evdev_device *device = NULL;
> int rc;
> int fd;
> int unhandled_device = 0;
> + const char *devnode = udev_device_get_devnode(udev_device);
>
> /* Use non-blocking mode so that we can loop on read on
> * evdev_device_data() until all events on the fd are
> @@ -1494,9 +1487,7 @@ evdev_device_create(struct libinput_seat *seat,
> device->seat_caps = 0;
> device->is_mt = 0;
> device->mtdev = NULL;
> - device->devnode = strdup(devnode);
> - device->sysname = strdup(sysname);
> - device->syspath = strdup(syspath);
> + device->udev_device = udev_device_ref(udev_device);
> device->rel.dx = 0;
> device->rel.dy = 0;
> device->abs.seat_slot = -1;
> @@ -1565,7 +1556,7 @@ evdev_device_get_output(struct evdev_device *device)
> const char *
> evdev_device_get_sysname(struct evdev_device *device)
> {
> - return device->sysname;
> + return udev_device_get_sysname(device->udev_device);
> }
>
> const char *
> @@ -1919,32 +1910,25 @@ evdev_device_suspend(struct evdev_device *device)
> }
>
> static int
> -evdev_device_compare_syspath(struct evdev_device *device, int fd)
> +evdev_device_compare_syspath(struct udev_device *udev_device, int fd)
> {
> - struct udev *udev = NULL;
> - struct udev_device *udev_device = NULL;
> - const char *syspath;
> + struct udev *udev = udev_device_get_udev(udev_device);
> + struct udev_device *udev_device_new;
> struct stat st;
> int rc = 1;
>
> - udev = udev_new();
> - if (!udev)
> - goto out;
> -
> if (fstat(fd, &st) < 0)
> goto out;
>
> - udev_device = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> - if (!device)
> + udev_device_new = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> + if (!udev_device_new)
> goto out;
>
> - syspath = udev_device_get_syspath(udev_device);
> - rc = strcmp(syspath, device->syspath);
> + rc = strcmp(udev_device_get_syspath(udev_device_new),
> + udev_device_get_syspath(udev_device));
> out:
> - if (udev_device)
> - udev_device_unref(udev_device);
> - if (udev)
> - udev_unref(udev);
> + if (udev_device_new)
> + udev_device_unref(udev_device_new);
> return rc;
> }
>
> @@ -1953,19 +1937,21 @@ evdev_device_resume(struct evdev_device *device)
> {
> struct libinput *libinput = device->base.seat->libinput;
> int fd;
> + const char *devnode;
>
> if (device->fd != -1)
> return 0;
>
> - if (device->syspath == NULL)
> + if (device->was_removed)
> return -ENODEV;
>
> - fd = open_restricted(libinput, device->devnode, O_RDWR | O_NONBLOCK);
> + devnode = udev_device_get_devnode(device->udev_device);
> + fd = open_restricted(libinput, devnode, O_RDWR | O_NONBLOCK);
>
> if (fd < 0)
> return -errno;
>
> - if (evdev_device_compare_syspath(device, fd)) {
> + if (evdev_device_compare_syspath(device->udev_device, fd)) {
> close_restricted(libinput, fd);
> return -ENODEV;
> }
> @@ -2008,10 +1994,9 @@ evdev_device_remove(struct evdev_device *device)
>
> evdev_device_suspend(device);
>
> - /* A device may be removed while suspended. Free the syspath to
> + /* A device may be removed while suspended, mark it to
> * skip re-opening a different device with the same node */
> - free(device->syspath);
> - device->syspath = NULL;
> + device->was_removed = true;
>
> list_remove(&device->base.link);
>
> @@ -2031,9 +2016,7 @@ evdev_device_destroy(struct evdev_device *device)
> filter_destroy(device->pointer.filter);
> libinput_seat_unref(device->base.seat);
> libevdev_free(device->evdev);
> + udev_device_unref(device->udev_device);
> free(device->mt.slots);
> - free(device->devnode);
> - free(device->sysname);
> - free(device->syspath);
> free(device);
> }
> diff --git a/src/evdev.h b/src/evdev.h
> index 35da319..0e8b445 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -68,11 +68,10 @@ struct evdev_device {
>
> struct evdev_dispatch *dispatch;
> struct libevdev *evdev;
> + struct udev_device *udev_device;
> char *output_name;
> - char *devnode;
> - char *sysname;
> - char *syspath;
> const char *devname;
> + bool was_removed;
> int fd;
> struct {
> const struct input_absinfo *absinfo_x, *absinfo_y;
> @@ -203,9 +202,7 @@ struct evdev_dispatch {
>
> struct evdev_device *
> evdev_device_create(struct libinput_seat *seat,
> - const char *devnode,
> - const char *sysname,
> - const char *syspath);
> + struct udev_device *device);
>
> int
> evdev_device_init_pointer_acceleration(struct evdev_device *device);
> diff --git a/src/path.c b/src/path.c
> index 175366b..9d0632a 100644
> --- a/src/path.c
> +++ b/src/path.c
> @@ -115,14 +115,10 @@ path_device_enable(struct path_input *input,
> {
> struct path_seat *seat;
> struct evdev_device *device = NULL;
> - char *sysname = NULL, *syspath = NULL;
> char *seat_name = NULL, *seat_logical_name = NULL;
> const char *seat_prop;
> const char *devnode;
>
> - sysname = strdup(udev_device_get_sysname(udev_device));
> - syspath = strdup(udev_device_get_syspath(udev_device));
> -
> seat_prop = udev_device_get_property_value(udev_device, "ID_SEAT");
> seat_name = strdup(seat_prop ? seat_prop : default_seat);
>
> @@ -144,7 +140,7 @@ path_device_enable(struct path_input *input,
> }
> }
>
> - device = evdev_device_create(&seat->base, devnode, sysname, syspath);
> + device = evdev_device_create(&seat->base, udev_device);
> libinput_seat_unref(&seat->base);
>
> if (device == EVDEV_UNHANDLED_DEVICE) {
> @@ -161,8 +157,6 @@ path_device_enable(struct path_input *input,
> }
>
> out:
> - free(sysname);
> - free(syspath);
> free(seat_name);
> free(seat_logical_name);
>
> @@ -313,8 +307,7 @@ libinput_path_remove_device(struct libinput_device *device)
> }
>
> list_for_each(dev, &input->path_list, link) {
> - const char *devnode = udev_device_get_devnode(dev->udev_device);
> - if (strcmp(evdev->devnode, devnode) == 0) {
> + if (dev->udev_device == evdev->udev_device) {
> list_remove(&dev->link);
> udev_device_unref(dev->udev_device);
> free(dev);
> diff --git a/src/udev-seat.c b/src/udev-seat.c
> index f2be66e..49c8f47 100644
> --- a/src/udev-seat.c
> +++ b/src/udev-seat.c
> @@ -46,8 +46,6 @@ device_added(struct udev_device *udev_device, struct udev_input *input)
> {
> struct evdev_device *device;
> const char *devnode;
> - const char *sysname;
> - const char *syspath;
> const char *device_seat, *seat_name, *output_name;
> const char *calibration_values;
> float calibration[6];
> @@ -61,8 +59,6 @@ device_added(struct udev_device *udev_device, struct udev_input *input)
> return 0;
>
> devnode = udev_device_get_devnode(udev_device);
> - sysname = udev_device_get_sysname(udev_device);
> - syspath = udev_device_get_syspath(udev_device);
>
> /* Search for matching logical seat */
> seat_name = udev_device_get_property_value(udev_device, "WL_SEAT");
> @@ -79,7 +75,7 @@ device_added(struct udev_device *udev_device, struct udev_input *input)
> return -1;
> }
>
> - device = evdev_device_create(&seat->base, devnode, sysname, syspath);
> + device = evdev_device_create(&seat->base, udev_device);
> libinput_seat_unref(&seat->base);
>
> if (device == EVDEV_UNHANDLED_DEVICE) {
> @@ -123,18 +119,20 @@ device_added(struct udev_device *udev_device, struct udev_input *input)
> static void
> device_removed(struct udev_device *udev_device, struct udev_input *input)
> {
> - const char *devnode;
> struct evdev_device *device, *next;
> struct udev_seat *seat;
> + const char *syspath;
>
> - devnode = udev_device_get_devnode(udev_device);
> + syspath = udev_device_get_syspath(udev_device);
> list_for_each(seat, &input->base.seat_list, base.link) {
> list_for_each_safe(device, next,
> &seat->base.devices_list, base.link) {
> - if (!strcmp(device->devnode, devnode)) {
> + if (!strcmp(syspath,
> + udev_device_get_syspath(device->udev_device))) {
> log_info(&input->base,
> "input device %s, %s removed\n",
> - device->devname, device->devnode);
> + device->devname,
> + udev_device_get_devnode(device->udev_device));
> evdev_device_remove(device);
> break;
> }
>
More information about the wayland-devel
mailing list