Re: [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path
From: David Herrmann <hidden>
Date: 2013-07-27 13:07:04
Also in:
lkml
Hi On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires [off-list ref] 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 [off-list ref] Cheers David
quoted hunk ↗ jump to hunk
[1] http://www.freedesktop.org/wiki/Evemu/ Signed-off-by: Benjamin Tissoires <redacted> --- 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
quoted hunk ↗ jump to hunk
* 0.3 09/04/2006 (Anssi Hannula [off-list ref]) * - 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()...
quoted hunk ↗ jump to hunk
+} + 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.
quoted hunk ↗ jump to hunk
+ 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_VERSIONdiff --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/