* Re: [PATCH] input: fix EVIOCGNAME regression
2009-07-13 17:11 ` Daniel Mack
@ 2009-07-13 20:45 ` Thadeu Lima de Souza Cascardo
2009-07-14 4:29 ` Dmitry Torokhov
2009-07-15 15:21 ` Dmitry Torokhov
2 siblings, 0 replies; 6+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2009-07-13 20:45 UTC (permalink / raw)
To: Daniel Mack; +Cc: Dmitry Torokhov, linux-input, s.neumann
[-- Attachment #1: Type: text/plain, Size: 4627 bytes --]
On Mon, Jul 13, 2009 at 07:11:30PM +0200, Daniel Mack wrote:
> On Mon, Jul 13, 2009 at 09:52:05AM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 13, 2009 at 06:39:11PM +0200, Daniel Mack wrote:
> > > Commit 3d5cb60e ("Input: simplify name handling for certain input
> > > handles") introduced a regression for the EVIOCGNAME ioctl.
> > >
> > > Before this, patch, the platform device's name was given back to
> > > userspace which was good to identify devices. After this patch, the
> > > device was ("event%d", minor) which is not descriptive at all.
> > >
> > > This fixes the behaviour by taking input_dev->name.
> > >
> > > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > > Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> > > Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > > Cc: Dmitry Torokhov <dtor@mail.ru>
> > > ---
> > > drivers/input/evdev.c | 6 +++---
> > > 1 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > > index 114efd8..c81e042 100644
> > > --- a/drivers/input/evdev.c
> > > +++ b/drivers/input/evdev.c
> > > @@ -608,8 +608,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > > p, compat_mode);
> > >
> > > if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> > > - return str_to_user(dev_name(&evdev->dev),
> > > - _IOC_SIZE(cmd), p);
> > > + return str_to_user(evdev->handle.name,
> > > + _IOC_SIZE(cmd), p);
> >
> > No, this actually should be just dev->name, handle names were always in
> > the form of eventX or jsX. The change below is not needed either.
> > Btw, there is the similar issue in joydev.c as well.
>
> Ok, how about the one below?
>
> Daniel
My bad. Those two reverts work for me.
Tested-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
I have also reviewed my own patch and understand what I did wrong and
that I did mess with the input device name instead of keeping only to
the event device naming. Regarding what I did intend to "fix" with the
patch, it is all OK. So, if you want to, you may also consider my
review. Although after this shame, you may not consider it worth enough.
:-D
Reviewed-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
>
>
> From 0c754925117421778bd9adf1bd8f4e818fc65c74 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Mon, 13 Jul 2009 18:33:53 +0200
> Subject: [PATCH] input: fix EVIOCGNAME/JSIOCGNAME regression
>
> Commit 3d5cb60e ("Input: simplify name handling for certain input
> handles") introduced a regression for the EVIOCGNAME/JSIOCGNAME
> ioctl.
>
> Before this, patch, the platform device's name was given back to
> userspace which was good to identify devices. After this patch, the
> device is ("event%d", minor) which is not descriptive at all.
>
> This fixes the behaviour by taking dev->name.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>
> ---
> drivers/input/evdev.c | 3 +--
> drivers/input/joydev.c | 7 +++----
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 114efd8..1148140 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -608,8 +608,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> p, compat_mode);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> - return str_to_user(dev_name(&evdev->dev),
> - _IOC_SIZE(cmd), p);
> + return str_to_user(dev->name, _IOC_SIZE(cmd), p);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGPHYS(0)))
> return str_to_user(dev->phys, _IOC_SIZE(cmd), p);
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 0e12f89..16397aa 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -536,14 +536,13 @@ static int joydev_ioctl_common(struct joydev *joydev,
> default:
> if ((cmd & ~IOCSIZE_MASK) == JSIOCGNAME(0)) {
> int len;
> - const char *name = dev_name(&dev->dev);
>
> - if (!name)
> + if (!dev->name)
> return 0;
> - len = strlen(name) + 1;
> + len = strlen(dev->name) + 1;
> if (len > _IOC_SIZE(cmd))
> len = _IOC_SIZE(cmd);
> - if (copy_to_user(argp, name, len))
> + if (copy_to_user(argp, dev->name, len))
> return -EFAULT;
> return len;
> }
> --
> 1.6.3.1
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] input: fix EVIOCGNAME regression
2009-07-13 17:11 ` Daniel Mack
2009-07-13 20:45 ` Thadeu Lima de Souza Cascardo
@ 2009-07-14 4:29 ` Dmitry Torokhov
2009-07-15 15:21 ` Dmitry Torokhov
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2009-07-14 4:29 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-input, s.neumann, Thadeu Lima de Souza Cascardo
On Mon, Jul 13, 2009 at 07:11:30PM +0200, Daniel Mack wrote:
> On Mon, Jul 13, 2009 at 09:52:05AM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 13, 2009 at 06:39:11PM +0200, Daniel Mack wrote:
> > > Commit 3d5cb60e ("Input: simplify name handling for certain input
> > > handles") introduced a regression for the EVIOCGNAME ioctl.
> > >
> > > Before this, patch, the platform device's name was given back to
> > > userspace which was good to identify devices. After this patch, the
> > > device was ("event%d", minor) which is not descriptive at all.
> > >
> > > This fixes the behaviour by taking input_dev->name.
> > >
> > > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > > Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> > > Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > > Cc: Dmitry Torokhov <dtor@mail.ru>
> > > ---
> > > drivers/input/evdev.c | 6 +++---
> > > 1 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > > index 114efd8..c81e042 100644
> > > --- a/drivers/input/evdev.c
> > > +++ b/drivers/input/evdev.c
> > > @@ -608,8 +608,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > > p, compat_mode);
> > >
> > > if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> > > - return str_to_user(dev_name(&evdev->dev),
> > > - _IOC_SIZE(cmd), p);
> > > + return str_to_user(evdev->handle.name,
> > > + _IOC_SIZE(cmd), p);
> >
> > No, this actually should be just dev->name, handle names were always in
> > the form of eventX or jsX. The change below is not needed either.
> > Btw, there is the similar issue in joydev.c as well.
>
> Ok, how about the one below?
>
> Daniel
>
>
> From 0c754925117421778bd9adf1bd8f4e818fc65c74 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Mon, 13 Jul 2009 18:33:53 +0200
> Subject: [PATCH] input: fix EVIOCGNAME/JSIOCGNAME regression
>
> Commit 3d5cb60e ("Input: simplify name handling for certain input
> handles") introduced a regression for the EVIOCGNAME/JSIOCGNAME
> ioctl.
>
> Before this, patch, the platform device's name was given back to
> userspace which was good to identify devices. After this patch, the
> device is ("event%d", minor) which is not descriptive at all.
>
> This fixes the behaviour by taking dev->name.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>
> ---
> drivers/input/evdev.c | 3 +--
> drivers/input/joydev.c | 7 +++----
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 114efd8..1148140 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -608,8 +608,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> p, compat_mode);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> - return str_to_user(dev_name(&evdev->dev),
> - _IOC_SIZE(cmd), p);
> + return str_to_user(dev->name, _IOC_SIZE(cmd), p);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGPHYS(0)))
> return str_to_user(dev->phys, _IOC_SIZE(cmd), p);
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 0e12f89..16397aa 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -536,14 +536,13 @@ static int joydev_ioctl_common(struct joydev *joydev,
> default:
> if ((cmd & ~IOCSIZE_MASK) == JSIOCGNAME(0)) {
> int len;
> - const char *name = dev_name(&dev->dev);
I like my temps so I'll just change it to "const char *name = dev->name"
here, otherwise applied to for-linus, thanks Daniel.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] input: fix EVIOCGNAME regression
2009-07-13 17:11 ` Daniel Mack
2009-07-13 20:45 ` Thadeu Lima de Souza Cascardo
2009-07-14 4:29 ` Dmitry Torokhov
@ 2009-07-15 15:21 ` Dmitry Torokhov
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2009-07-15 15:21 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-input, s.neumann, Thadeu Lima de Souza Cascardo
On Mon, Jul 13, 2009 at 07:11:30PM +0200, Daniel Mack wrote:
> On Mon, Jul 13, 2009 at 09:52:05AM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 13, 2009 at 06:39:11PM +0200, Daniel Mack wrote:
> > > Commit 3d5cb60e ("Input: simplify name handling for certain input
> > > handles") introduced a regression for the EVIOCGNAME ioctl.
> > >
> > > Before this, patch, the platform device's name was given back to
> > > userspace which was good to identify devices. After this patch, the
> > > device was ("event%d", minor) which is not descriptive at all.
> > >
> > > This fixes the behaviour by taking input_dev->name.
> > >
> > > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > > Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> > > Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > > Cc: Dmitry Torokhov <dtor@mail.ru>
> > > ---
> > > drivers/input/evdev.c | 6 +++---
> > > 1 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > > index 114efd8..c81e042 100644
> > > --- a/drivers/input/evdev.c
> > > +++ b/drivers/input/evdev.c
> > > @@ -608,8 +608,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > > p, compat_mode);
> > >
> > > if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> > > - return str_to_user(dev_name(&evdev->dev),
> > > - _IOC_SIZE(cmd), p);
> > > + return str_to_user(evdev->handle.name,
> > > + _IOC_SIZE(cmd), p);
> >
> > No, this actually should be just dev->name, handle names were always in
> > the form of eventX or jsX. The change below is not needed either.
> > Btw, there is the similar issue in joydev.c as well.
>
> Ok, how about the one below?
>
> Daniel
>
>
> From 0c754925117421778bd9adf1bd8f4e818fc65c74 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Mon, 13 Jul 2009 18:33:53 +0200
> Subject: [PATCH] input: fix EVIOCGNAME/JSIOCGNAME regression
>
> Commit 3d5cb60e ("Input: simplify name handling for certain input
> handles") introduced a regression for the EVIOCGNAME/JSIOCGNAME
> ioctl.
>
> Before this, patch, the platform device's name was given back to
> userspace which was good to identify devices. After this patch, the
> device is ("event%d", minor) which is not descriptive at all.
>
> This fixes the behaviour by taking dev->name.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> Cc: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>
> ---
> drivers/input/evdev.c | 3 +--
> drivers/input/joydev.c | 7 +++----
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 114efd8..1148140 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -608,8 +608,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> p, compat_mode);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGNAME(0)))
> - return str_to_user(dev_name(&evdev->dev),
> - _IOC_SIZE(cmd), p);
> + return str_to_user(dev->name, _IOC_SIZE(cmd), p);
>
> if (_IOC_NR(cmd) == _IOC_NR(EVIOCGPHYS(0)))
> return str_to_user(dev->phys, _IOC_SIZE(cmd), p);
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 0e12f89..16397aa 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -536,14 +536,13 @@ static int joydev_ioctl_common(struct joydev *joydev,
> default:
> if ((cmd & ~IOCSIZE_MASK) == JSIOCGNAME(0)) {
> int len;
> - const char *name = dev_name(&dev->dev);
I like my temps so I'll just change it to "const char *name = dev->name"
here, otherwise applied to for-linus, thanks Daniel.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread