[PATCH 11/12] config-udev: Refactor input device addition for delayed input device probing

Peter Hutterer peter.hutterer at who-t.net
Tue Jan 28 16:42:07 PST 2014


On Wed, Jan 15, 2014 at 03:32:25PM +0100, Hans de Goede wrote:
> With systemd-logind we need to delay input device probing when switched away
> (iow not on the active vt). This is a preparation patch for this.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  config/udev.c | 150 +++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 85 insertions(+), 65 deletions(-)
> 
> diff --git a/config/udev.c b/config/udev.c
> index 9579bf2..58bc3d9 100644
> --- a/config/udev.c
> +++ b/config/udev.c
> @@ -52,6 +52,12 @@
>                     "returned \"%s\"\n",                                 \
>                     (attr), (path), (val) ? (val) : "(null)")
>  
> +struct input_device_info {
> +    InputOption *options;
> +    InputAttributes attrs;
> +    dev_t devnum;
> +};
> +
>  static struct udev_monitor *udev_monitor;
>  
>  #ifdef CONFIG_UDEV_KMS
> @@ -70,6 +76,48 @@ static const char *itoa(int i)
>  }
>  
>  static void
> +add_input_device(struct input_device_info *input, int fd)
> +{
> +    DeviceIntPtr dev = NULL;
> +    int rc;
> +
> +    if (fd != -1)
> +        input->options = input_option_new(input->options, "fd", itoa(fd));

move this into the caller.

> +
> +    LogMessage(X_INFO, "config/udev: Adding input device %s (%s:%d)\n",
> +               input->attrs.product, input->attrs.device, fd);
> +
> +    rc = NewInputDeviceRequest(input->options, &input->attrs, &dev);
> +    if (rc != Success) {
> +        if (fd != -1) {
> +            systemd_logind_put_fd(input->devnum);
> +            close(fd);
> +        }
> +    }
> +}
> +
> +static void
> +free_input_device(struct input_device_info *input)
> +{
> +    input_option_free_list(&input->options);
> +
> +    free((void *) input->attrs.usb_id);
> +    free((void *) input->attrs.pnp_id);
> +    free((void *) input->attrs.product);
> +    free((void *) input->attrs.device);
> +    free((void *) input->attrs.vendor);

whoah, when did this stuff become const? I might need to undo this first,
looks like fecc7eb1cf66db64728ee2d68cd9443df7e70879 was a bit eager there.

> +    if (input->attrs.tags) {
> +        const char **tag = input->attrs.tags;
> +
> +        while (*tag) {
> +            free((void *) *tag);
> +            tag++;
> +        }
> +        free(input->attrs.tags);
> +    }
> +}
> +
> +static void
>  device_added(struct udev_device *udev_device)
>  {
>      const char *path, *name = NULL;
> @@ -77,12 +125,10 @@ device_added(struct udev_device *udev_device)
>      const char *syspath;
>      const char *tags_prop;
>      const char *key, *value, *tmp;
> -    InputOption *input_options;
> -    InputAttributes attrs = { };
> -    DeviceIntPtr dev = NULL;
> +    struct input_device_info input = { NULL, { } };

this patch would be a lot simpler if you just leave the input_options, attrs
in place and before the call to add_input_device() merely do a

input.options = input_options;
input.attrs  = attrs;
input.devnum = devnum;
add_input_device(&attrs, fd);

Cheers,
   Peter

>      struct udev_list_entry *set, *entry;
>      struct udev_device *parent;
> -    int rc, fd = -1;
> +    int fd = -1;
>      const char *dev_seat;
>      dev_t devnum;
>      Bool paused = FALSE;
> @@ -131,8 +177,9 @@ device_added(struct udev_device *udev_device)
>          return;
>      }
>  
> -    input_options = input_option_new(NULL, "_source", "server/udev");
> -    if (!input_options)
> +    input.devnum = devnum;
> +    input.options = input_option_new(NULL, "_source", "server/udev");
> +    if (!input.options)
>          return;
>  
>      parent = udev_device_get_parent(udev_device);
> @@ -150,7 +197,7 @@ device_added(struct udev_device *udev_device)
>          }
>  
>          if (pnp_id)
> -            attrs.pnp_id = strdup(pnp_id);
> +            input.attrs.pnp_id = strdup(pnp_id);
>          LOG_SYSATTR(ppath, "id", pnp_id);
>  
>          /* construct USB ID in lowercase hex - "0000:ffff" */
> @@ -162,24 +209,24 @@ device_added(struct udev_device *udev_device)
>                  usb_id = NULL;
>              else
>                  LOG_PROPERTY(ppath, "PRODUCT", product);
> -            attrs.usb_id = usb_id;
> +            input.attrs.usb_id = usb_id;
>          }
>      }
>      if (!name)
>          name = "(unnamed)";
>      else
> -        attrs.product = strdup(name);
> -    input_options = input_option_new(input_options, "name", name);
> -    input_options = input_option_new(input_options, "path", path);
> -    input_options = input_option_new(input_options, "device", path);
> -    input_options = input_option_new(input_options, "major", itoa(major(devnum)));
> -    input_options = input_option_new(input_options, "minor", itoa(minor(devnum)));
> +        input.attrs.product = strdup(name);
> +    input.options = input_option_new(input.options, "name", name);
> +    input.options = input_option_new(input.options, "path", path);
> +    input.options = input_option_new(input.options, "device", path);
> +    input.options = input_option_new(input.options, "major", itoa(major(devnum)));
> +    input.options = input_option_new(input.options, "minor", itoa(minor(devnum)));
>      if (path)
> -        attrs.device = strdup(path);
> +        input.attrs.device = strdup(path);
>  
>      tags_prop = udev_device_get_property_value(udev_device, "ID_INPUT.tags");
>      LOG_PROPERTY(path, "ID_INPUT.tags", tags_prop);
> -    attrs.tags = xstrtokenize(tags_prop, ",");
> +    input.attrs.tags = xstrtokenize(tags_prop, ",");
>  
>      if (asprintf(&config_info, "udev:%s", syspath) == -1) {
>          config_info = NULL;
> @@ -202,94 +249,67 @@ device_added(struct udev_device *udev_device)
>              LOG_PROPERTY(path, key, value);
>              tmp = key + sizeof(UDEV_XKB_PROP_KEY) - 1;
>              if (!strcasecmp(tmp, "rules"))
> -                input_options =
> -                    input_option_new(input_options, "xkb_rules", value);
> +                input.options =
> +                    input_option_new(input.options, "xkb_rules", value);
>              else if (!strcasecmp(tmp, "layout"))
> -                input_options =
> -                    input_option_new(input_options, "xkb_layout", value);
> +                input.options =
> +                    input_option_new(input.options, "xkb_layout", value);
>              else if (!strcasecmp(tmp, "variant"))
> -                input_options =
> -                    input_option_new(input_options, "xkb_variant", value);
> +                input.options =
> +                    input_option_new(input.options, "xkb_variant", value);
>              else if (!strcasecmp(tmp, "model"))
> -                input_options =
> -                    input_option_new(input_options, "xkb_model", value);
> +                input.options =
> +                    input_option_new(input.options, "xkb_model", value);
>              else if (!strcasecmp(tmp, "options"))
> -                input_options =
> -                    input_option_new(input_options, "xkb_options", value);
> +                input.options =
> +                    input_option_new(input.options, "xkb_options", value);
>          }
>          else if (!strcmp(key, "ID_VENDOR")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.vendor = strdup(value);
> +            input.attrs.vendor = strdup(value);
>          }
>          else if (!strcmp(key, "ID_INPUT_KEY")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_KEYBOARD;
> +            input.attrs.flags |= ATTR_KEYBOARD;
>          }
>          else if (!strcmp(key, "ID_INPUT_MOUSE")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_POINTER;
> +            input.attrs.flags |= ATTR_POINTER;
>          }
>          else if (!strcmp(key, "ID_INPUT_JOYSTICK")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_JOYSTICK;
> +            input.attrs.flags |= ATTR_JOYSTICK;
>          }
>          else if (!strcmp(key, "ID_INPUT_TABLET")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_TABLET;
> +            input.attrs.flags |= ATTR_TABLET;
>          }
>          else if (!strcmp(key, "ID_INPUT_TOUCHPAD")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_TOUCHPAD;
> +            input.attrs.flags |= ATTR_TOUCHPAD;
>          }
>          else if (!strcmp(key, "ID_INPUT_TOUCHSCREEN")) {
>              LOG_PROPERTY(path, key, value);
> -            attrs.flags |= ATTR_TOUCHSCREEN;
> +            input.attrs.flags |= ATTR_TOUCHSCREEN;
>          }
>      }
>  
> -    input_options = input_option_new(input_options, "config_info", config_info);
> +    input.options = input_option_new(input.options, "config_info", config_info);
>  
>      /* Default setting needed for non-seat0 seats */
>      if (ServerIsNotSeat0())
> -        input_options = input_option_new(input_options, "GrabDevice", "on");
> +        input.options = input_option_new(input.options, "GrabDevice", "on");
>  
>      fd = systemd_logind_get_fd(devnum, path, &paused);
> -    if (fd != -1)
> -        input_options = input_option_new(input_options, "fd", itoa(fd));
>  
> -    LogMessage(X_INFO, "config/udev: Adding input device %s (%s:%d)\n",
> -               name, path, fd);
>      /* FIXME check paused, if paused put the device on a list for probing
>         later */
> -    rc = NewInputDeviceRequest(input_options, &attrs, &dev);
> -    if (rc != Success) {
> -        if (fd != -1) {
> -            systemd_logind_put_fd(devnum);
> -            close(fd);
> -        }
> -        goto unwind;
> -    }
> +
> +    add_input_device(&input, fd);
>  
>   unwind:
>      free(config_info);
> -    input_option_free_list(&input_options);
> -
> -    free((void *) attrs.usb_id);
> -    free((void *) attrs.pnp_id);
> -    free((void *) attrs.product);
> -    free((void *) attrs.device);
> -    free((void *) attrs.vendor);
> -    if (attrs.tags) {
> -        const char **tag = attrs.tags;
> -
> -        while (*tag) {
> -            free((void *) *tag);
> -            tag++;
> -        }
> -        free(attrs.tags);
> -    }
> -
> -    return;
> +    free_input_device(&input);
>  }
>  
>  static void
> -- 
> 1.8.4.2
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 


More information about the xorg-devel mailing list