* [PATCH v2 1/2] input/uinput: support abs resolution @ 2013-07-15 13:37 Benjamin Tissoires 2013-07-15 13:37 ` [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path Benjamin Tissoires 0 siblings, 1 reply; 5+ messages in thread From: Benjamin Tissoires @ 2013-07-15 13:37 UTC (permalink / raw) To: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, linux-input, linux-kernel From: Peter Hutterer <peter.hutterer@who-t.net> Input resolution has been added in kernel v2.6.31 with commit ec20a022aa24fc63. However, uinput was never updated since, meaning that devices cannot be emulated in full. User-space processes that rely on the resolution to be accurate need this. Resolution is added in struct uinput_user_dev in a way it won't break current programs relying on the old implementation. Bump the UINPUT_VERSION number to 4 to aid detection of this in user-space. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> --- Sorry for that, I mixed up the "From:" field. This v2 contains the right From in the first patch , and this is the only difference... Cheers, Benjamin drivers/input/misc/uinput.c | 17 ++++++++++++----- include/linux/uinput.h | 2 ++ include/uapi/linux/uinput.h | 3 +++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index a0a4bba..7d518b4 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -20,6 +20,8 @@ * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> * * Changes/Revisions: + * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) + * - update uinput_user_dev struct to allow abs resolution * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>) * - updated ff support for the changes in kernel interface * - added MODULE_VERSION @@ -364,9 +366,9 @@ static int uinput_setup_device(struct uinput_device *udev, struct input_dev *dev; int i; int retval; + size_t size; - if (count != sizeof(struct uinput_user_dev)) - return -EINVAL; + size = min_t(size_t, count, sizeof(struct uinput_user_dev)); if (!udev->dev) { retval = uinput_allocate_device(udev); @@ -376,9 +378,13 @@ static int uinput_setup_device(struct uinput_device *udev, dev = udev->dev; - user_dev = memdup_user(buffer, sizeof(struct uinput_user_dev)); - if (IS_ERR(user_dev)) - return PTR_ERR(user_dev); + user_dev = kzalloc(sizeof(struct uinput_user_dev), GFP_KERNEL); + if (!user_dev) + return -ENOMEM; + if (copy_from_user(user_dev, buffer, size)) { + retval = -EFAULT; + goto exit; + } udev->ff_effects_max = user_dev->ff_effects_max; @@ -406,6 +412,7 @@ static int uinput_setup_device(struct uinput_device *udev, input_abs_set_min(dev, i, user_dev->absmin[i]); input_abs_set_fuzz(dev, i, user_dev->absfuzz[i]); input_abs_set_flat(dev, i, user_dev->absflat[i]); + input_abs_set_res(dev, i, user_dev->absres[i]); } /* check if absmin/absmax/absfuzz/absflat are filled as diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 0a4487d..6291a22 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -20,6 +20,8 @@ * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> * * Changes/Revisions: + * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) + * - update uinput_user_dev struct to allow abs resolution * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) * - update ff support for the changes in kernel interface * - add UINPUT_VERSION diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h index fe46431..f6a393b 100644 --- a/include/uapi/linux/uinput.h +++ b/include/uapi/linux/uinput.h @@ -20,6 +20,8 @@ * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org> * * Changes/Revisions: + * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) + * - update uinput_user_dev struct to allow abs resolution * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) * - update ff support for the changes in kernel interface * - add UINPUT_VERSION @@ -133,5 +135,6 @@ struct uinput_user_dev { __s32 absmin[ABS_CNT]; __s32 absfuzz[ABS_CNT]; __s32 absflat[ABS_CNT]; + __s32 absres[ABS_CNT]; }; #endif /* _UAPI__UINPUT_H_ */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path 2013-07-15 13:37 [PATCH v2 1/2] input/uinput: support abs resolution Benjamin Tissoires @ 2013-07-15 13:37 ` Benjamin Tissoires 2013-07-23 1:15 ` Peter Hutterer 2013-07-27 13:07 ` David Herrmann 0 siblings, 2 replies; 5+ messages in thread From: Benjamin Tissoires @ 2013-07-15 13:37 UTC (permalink / raw) To: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, linux-input, linux-kernel Cc: Benjamin Tissoires Evemu [1] uses uinput to replay devices traces it has recorded. However, the way evemu uses uinput is slightly different from how uinput is supposed to be used. Evemu creates the device node through uinput, bu inject events through the input device node directly (and skipping the uinput node). Currently, evemu relies on an heuristic to guess which input node was created. The problem is that is heuristic is subjected to races between different uinput devices or even with physical devices. Having a way to retrieve the sysfs path allows us to find the event node without having to rely on this heuristic. [1] http://www.freedesktop.org/wiki/Evemu/ Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++- include/linux/uinput.h | 1 + include/uapi/linux/uinput.h | 3 +++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 7d518b4..49a9f7d 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -22,6 +22,7 @@ * Changes/Revisions: * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) * - update uinput_user_dev struct to allow abs resolution + * - add UI_GET_SYSPATH ioctl * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>) * - updated ff support for the changes in kernel interface * - added MODULE_VERSION @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer, __ret; \ }) +static int uinput_str_to_user(const char *str, unsigned int maxlen, + void __user *p) +{ + int len; + + if (!str) + return -ENOENT; + + len = strlen(str) + 1; + if (len > maxlen) + len = maxlen; + + return copy_to_user(p, str, len) ? -EFAULT : len; +} + static long uinput_ioctl_handler(struct file *file, unsigned int cmd, unsigned long arg, void __user *p) { @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, struct uinput_ff_erase ff_erase; struct uinput_request *req; char *phys; + const char *path; + unsigned int size; retval = mutex_lock_interruptible(&udev->mutex); if (retval) @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, break; default: - retval = -EINVAL; + retval = -EAGAIN; + } + + if (retval == -EAGAIN) { + size = _IOC_SIZE(cmd); + + /* Now check variable-length commands */ + switch (cmd & ~IOCSIZE_MASK) { + case UI_GET_SYSPATH(0): + path = kobject_get_path(&udev->dev->dev.kobj, + GFP_KERNEL); + retval = uinput_str_to_user(path, size, p); + kfree(path); + break; + + default: + retval = -EINVAL; + } } out: diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 6291a22..64fab81 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -22,6 +22,7 @@ * Changes/Revisions: * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) * - update uinput_user_dev struct to allow abs resolution + * - add UI_GET_SYSPATH ioctl * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) * - update ff support for the changes in kernel interface * - add UINPUT_VERSION diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h index f6a393b..d826409 100644 --- a/include/uapi/linux/uinput.h +++ b/include/uapi/linux/uinput.h @@ -22,6 +22,7 @@ * Changes/Revisions: * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) * - update uinput_user_dev struct to allow abs resolution + * - add UI_GET_SYSPATH ioctl * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) * - update ff support for the changes in kernel interface * - add UINPUT_VERSION @@ -75,6 +76,8 @@ struct uinput_ff_erase { #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase) #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase) +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len) + /* * To write a force-feedback-capable driver, the upload_effect * and erase_effect callbacks in input_dev must be implemented. -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path 2013-07-15 13:37 ` [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path Benjamin Tissoires @ 2013-07-23 1:15 ` Peter Hutterer 2013-07-27 13:07 ` David Herrmann 1 sibling, 0 replies; 5+ messages in thread From: Peter Hutterer @ 2013-07-23 1:15 UTC (permalink / raw) To: Benjamin Tissoires Cc: Dmitry Torokhov, Benjamin Tissoires, linux-input, linux-kernel On Mon, Jul 15, 2013 at 03:37:09PM +0200, Benjamin Tissoires wrote: > Evemu [1] uses uinput to replay devices traces it has recorded. However, > the way evemu uses uinput is slightly different from how uinput is > supposed to be used. > Evemu creates the device node through uinput, bu inject events through > the input device node directly (and skipping the uinput node). > > Currently, evemu relies on an heuristic to guess which input node was > created. The problem is that is heuristic is subjected to races between > different uinput devices or even with physical devices. Having a way > to retrieve the sysfs path allows us to find the event node without > having to rely on this heuristic. > > [1] http://www.freedesktop.org/wiki/Evemu/ > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Tested-by: Peter Hutterer <peter.hutterer@who-t.net> I'd really appreciate a comment above the ioctl define stating what the ioctl actually returns. only comment regarding the code: > +static int uinput_str_to_user(const char *str, unsigned int maxlen, > + void __user *p) I'd do this with a signature of the type (dest, src, len). Cheers, Peter > --- > drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++- > include/linux/uinput.h | 1 + > include/uapi/linux/uinput.h | 3 +++ > 3 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > index 7d518b4..49a9f7d 100644 > --- a/drivers/input/misc/uinput.c > +++ b/drivers/input/misc/uinput.c > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl > * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>) > * - updated ff support for the changes in kernel interface > * - added MODULE_VERSION > @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer, > __ret; \ > }) > > +static int uinput_str_to_user(const char *str, unsigned int maxlen, > + void __user *p) > +{ > + int len; > + > + if (!str) > + return -ENOENT; > + > + len = strlen(str) + 1; > + if (len > maxlen) > + len = maxlen; > + > + return copy_to_user(p, str, len) ? -EFAULT : len; > +} > + > static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > unsigned long arg, void __user *p) > { > @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > struct uinput_ff_erase ff_erase; > struct uinput_request *req; > char *phys; > + const char *path; > + unsigned int size; > > retval = mutex_lock_interruptible(&udev->mutex); > if (retval) > @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > break; > > default: > - retval = -EINVAL; > + retval = -EAGAIN; > + } > + > + if (retval == -EAGAIN) { > + size = _IOC_SIZE(cmd); > + > + /* Now check variable-length commands */ > + switch (cmd & ~IOCSIZE_MASK) { > + case UI_GET_SYSPATH(0): > + path = kobject_get_path(&udev->dev->dev.kobj, > + GFP_KERNEL); > + retval = uinput_str_to_user(path, size, p); > + kfree(path); > + break; > + > + default: > + retval = -EINVAL; > + } > } > > out: > diff --git a/include/linux/uinput.h b/include/linux/uinput.h > index 6291a22..64fab81 100644 > --- a/include/linux/uinput.h > +++ b/include/linux/uinput.h > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > * - update ff support for the changes in kernel interface > * - add UINPUT_VERSION > diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h > index f6a393b..d826409 100644 > --- a/include/uapi/linux/uinput.h > +++ b/include/uapi/linux/uinput.h > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > * - update ff support for the changes in kernel interface > * - add UINPUT_VERSION > @@ -75,6 +76,8 @@ struct uinput_ff_erase { > #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase) > #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase) > > +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len) > + > /* > * To write a force-feedback-capable driver, the upload_effect > * and erase_effect callbacks in input_dev must be implemented. > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path 2013-07-15 13:37 ` [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path Benjamin Tissoires 2013-07-23 1:15 ` Peter Hutterer @ 2013-07-27 13:07 ` David Herrmann 2013-07-29 9:16 ` Benjamin Tissoires 1 sibling, 1 reply; 5+ messages in thread From: David Herrmann @ 2013-07-27 13:07 UTC (permalink / raw) To: Benjamin Tissoires Cc: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, open list:HID CORE LAYER, linux-kernel Hi On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > Evemu [1] uses uinput to replay devices traces it has recorded. However, > the way evemu uses uinput is slightly different from how uinput is > supposed to be used. > Evemu creates the device node through uinput, bu inject events through > the input device node directly (and skipping the uinput node). > > Currently, evemu relies on an heuristic to guess which input node was > created. The problem is that is heuristic is subjected to races between > different uinput devices or even with physical devices. Having a way > to retrieve the sysfs path allows us to find the event node without > having to rely on this heuristic. It would actually be enough to return the "input_no" from input_register_device() (which is currently local but we could save it in "dev"). Or only the device-name. I don't know why you want the full syspath. It's just overhead in the kernel that we could easily let user-space do. And the path /sys/class/input/<devname> can be put together by user-space. Anyway, Dmitry has to decide on that. Apart from some style-issues I mentioned below: Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Cheers David > [1] http://www.freedesktop.org/wiki/Evemu/ > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++- > include/linux/uinput.h | 1 + > include/uapi/linux/uinput.h | 3 +++ > 3 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > index 7d518b4..49a9f7d 100644 > --- a/drivers/input/misc/uinput.c > +++ b/drivers/input/misc/uinput.c > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl What tree is that patch against? I cannot see an "0.4" entry in: http://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/tree/drivers/input/misc/uinput.c?h=next > * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>) > * - updated ff support for the changes in kernel interface > * - added MODULE_VERSION > @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer, > __ret; \ > }) > > +static int uinput_str_to_user(const char *str, unsigned int maxlen, > + void __user *p) As Peter mentioned, I'd move "maxlen" to the end. > +{ > + int len; > + > + if (!str) > + return -ENOENT; > + > + len = strlen(str) + 1; > + if (len > maxlen) > + len = maxlen; > + > + return copy_to_user(p, str, len) ? -EFAULT : len; I'd prefer a "strlcpy()" so we guarantee a terminating 0 for user-space, but I guess that'd be rather complex to do here. I couldn't find any strlcpy_to_user()... > +} > + > static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > unsigned long arg, void __user *p) > { > @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > struct uinput_ff_erase ff_erase; > struct uinput_request *req; > char *phys; > + const char *path; > + unsigned int size; > > retval = mutex_lock_interruptible(&udev->mutex); > if (retval) > @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > break; > > default: > - retval = -EINVAL; > + retval = -EAGAIN; > + } > + > + if (retval == -EAGAIN) { > + size = _IOC_SIZE(cmd); > + > + /* Now check variable-length commands */ > + switch (cmd & ~IOCSIZE_MASK) { > + case UI_GET_SYSPATH(0): > + path = kobject_get_path(&udev->dev->dev.kobj, > + GFP_KERNEL); I know, device registration is protected by udev->mutex but I'd still prefer: if (udev->state != UIST_CREATED) return -ENOENT; But that's probably a matter of taste. > + retval = uinput_str_to_user(path, size, p); > + kfree(path); > + break; > + > + default: > + retval = -EINVAL; > + } > } > > out: > diff --git a/include/linux/uinput.h b/include/linux/uinput.h > index 6291a22..64fab81 100644 > --- a/include/linux/uinput.h > +++ b/include/linux/uinput.h > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > * - update ff support for the changes in kernel interface > * - add UINPUT_VERSION > diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h > index f6a393b..d826409 100644 > --- a/include/uapi/linux/uinput.h > +++ b/include/uapi/linux/uinput.h > @@ -22,6 +22,7 @@ > * Changes/Revisions: > * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) > * - update uinput_user_dev struct to allow abs resolution > + * - add UI_GET_SYSPATH ioctl > * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) > * - update ff support for the changes in kernel interface > * - add UINPUT_VERSION > @@ -75,6 +76,8 @@ struct uinput_ff_erase { > #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase) > #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase) > > +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len) > + > /* > * To write a force-feedback-capable driver, the upload_effect > * and erase_effect callbacks in input_dev must be implemented. > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path 2013-07-27 13:07 ` David Herrmann @ 2013-07-29 9:16 ` Benjamin Tissoires 0 siblings, 0 replies; 5+ messages in thread From: Benjamin Tissoires @ 2013-07-29 9:16 UTC (permalink / raw) To: David Herrmann Cc: Benjamin Tissoires, Dmitry Torokhov, Peter Hutterer, open list:HID CORE LAYER, linux-kernel Hi David, On Sat, Jul 27, 2013 at 3:07 PM, David Herrmann <dh.herrmann@gmail.com> wrote: > Hi > > On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: >> Evemu [1] uses uinput to replay devices traces it has recorded. However, >> the way evemu uses uinput is slightly different from how uinput is >> supposed to be used. >> Evemu creates the device node through uinput, bu inject events through >> the input device node directly (and skipping the uinput node). >> >> Currently, evemu relies on an heuristic to guess which input node was >> created. The problem is that is heuristic is subjected to races between >> different uinput devices or even with physical devices. Having a way >> to retrieve the sysfs path allows us to find the event node without >> having to rely on this heuristic. > > It would actually be enough to return the "input_no" from > input_register_device() (which is currently local but we could save it > in "dev"). Or only the device-name. I don't know why you want the full > syspath. It's just overhead in the kernel that we could easily let > user-space do. And the path /sys/class/input/<devname> can be put > together by user-space. I agree that the "input_no" is enough, but that would infer a change in the input struct. However, we (Peter and I) want to be able to use it in RHEL, or any other legacy distribution, where it is not acceptable to ask to recompile a new kernel to do regressions tests. For those distributions, I have set up a backport of uinput [2] and with this backport, you only need to recompile the uinput module (not the whole kernel tree). Anyway, returning the input name seems enough too. It will remove the path retrieval and a free :) I agree this is not the kind of argument we usually like in the kernel tree, but as this feature (resolution and input retrieval) seems to be only valuable for regression tests (Xorg or Wayland), I think it is valuable. > > Anyway, Dmitry has to decide on that. Apart from some style-issues I > mentioned below: > Reviewed-by: David Herrmann <dh.herrmann@gmail.com> thanks! > > Cheers > David > >> [1] http://www.freedesktop.org/wiki/Evemu/ [2] https://github.com/bentiss/uinput >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++- >> include/linux/uinput.h | 1 + >> include/uapi/linux/uinput.h | 3 +++ >> 3 files changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c >> index 7d518b4..49a9f7d 100644 >> --- a/drivers/input/misc/uinput.c >> +++ b/drivers/input/misc/uinput.c >> @@ -22,6 +22,7 @@ >> * Changes/Revisions: >> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) >> * - update uinput_user_dev struct to allow abs resolution >> + * - add UI_GET_SYSPATH ioctl > > What tree is that patch against? I cannot see an "0.4" entry in: > http://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/tree/drivers/input/misc/uinput.c?h=next looks like you miss the msg title: 2/2. It is part of a series where the first patch was done by Peter here: https://patchwork.kernel.org/patch/2827521/ > >> * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>) >> * - updated ff support for the changes in kernel interface >> * - added MODULE_VERSION >> @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer, >> __ret; \ >> }) >> >> +static int uinput_str_to_user(const char *str, unsigned int maxlen, >> + void __user *p) > > As Peter mentioned, I'd move "maxlen" to the end. right. I just copied/pasted the code in evdev.c, but this can be easily changed. > >> +{ >> + int len; >> + >> + if (!str) >> + return -ENOENT; >> + >> + len = strlen(str) + 1; >> + if (len > maxlen) >> + len = maxlen; >> + >> + return copy_to_user(p, str, len) ? -EFAULT : len; > > I'd prefer a "strlcpy()" so we guarantee a terminating 0 for > user-space, but I guess that'd be rather complex to do here. I > couldn't find any strlcpy_to_user()... Definitively, adding a terminating 0 seems a good idea to prevent leaking of information from the kernel. > >> +} >> + >> static long uinput_ioctl_handler(struct file *file, unsigned int cmd, >> unsigned long arg, void __user *p) >> { >> @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, >> struct uinput_ff_erase ff_erase; >> struct uinput_request *req; >> char *phys; >> + const char *path; >> + unsigned int size; >> >> retval = mutex_lock_interruptible(&udev->mutex); >> if (retval) >> @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, >> break; >> >> default: >> - retval = -EINVAL; >> + retval = -EAGAIN; >> + } >> + >> + if (retval == -EAGAIN) { >> + size = _IOC_SIZE(cmd); >> + >> + /* Now check variable-length commands */ >> + switch (cmd & ~IOCSIZE_MASK) { >> + case UI_GET_SYSPATH(0): >> + path = kobject_get_path(&udev->dev->dev.kobj, >> + GFP_KERNEL); > > I know, device registration is protected by udev->mutex but I'd still prefer: > if (udev->state != UIST_CREATED) > return -ENOENT; oops. Definitively, this should be added. > > But that's probably a matter of taste. I think it is a mistake not adding this. Without it (in the current implementation), the user-space can call it before the device is created, leading to some garbage (or even oopses) depending on how kobject_get_path() handles the queries for non added devices. Cheers, Benjamin > >> + retval = uinput_str_to_user(path, size, p); >> + kfree(path); >> + break; >> + >> + default: >> + retval = -EINVAL; >> + } >> } >> >> out: >> diff --git a/include/linux/uinput.h b/include/linux/uinput.h >> index 6291a22..64fab81 100644 >> --- a/include/linux/uinput.h >> +++ b/include/linux/uinput.h >> @@ -22,6 +22,7 @@ >> * Changes/Revisions: >> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) >> * - update uinput_user_dev struct to allow abs resolution >> + * - add UI_GET_SYSPATH ioctl >> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) >> * - update ff support for the changes in kernel interface >> * - add UINPUT_VERSION >> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h >> index f6a393b..d826409 100644 >> --- a/include/uapi/linux/uinput.h >> +++ b/include/uapi/linux/uinput.h >> @@ -22,6 +22,7 @@ >> * Changes/Revisions: >> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>) >> * - update uinput_user_dev struct to allow abs resolution >> + * - add UI_GET_SYSPATH ioctl >> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>) >> * - update ff support for the changes in kernel interface >> * - add UINPUT_VERSION >> @@ -75,6 +76,8 @@ struct uinput_ff_erase { >> #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase) >> #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase) >> >> +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len) >> + >> /* >> * To write a force-feedback-capable driver, the upload_effect >> * and erase_effect callbacks in input_dev must be implemented. >> -- >> 1.8.3.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-29 9:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-15 13:37 [PATCH v2 1/2] input/uinput: support abs resolution Benjamin Tissoires 2013-07-15 13:37 ` [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path Benjamin Tissoires 2013-07-23 1:15 ` Peter Hutterer 2013-07-27 13:07 ` David Herrmann 2013-07-29 9:16 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).