* [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
@ 2010-12-09 9:39 Dmitry Torokhov
2010-12-09 19:04 ` Henrik Rydberg
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2010-12-09 9:39 UTC (permalink / raw)
To: Linux Input
Cc: LKML, linux-media, Mauro Carvalho Chehab, Jiri Kosina,
Jarod Wilson, David Härdeman, Henrik Rydberg
The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
extending them to support large scancodes was a mistake. While we tried
to keep ABI intact (and we succeeded in doing that, programs compiled
on older kernels will work on newer ones) there is still a problem with
recompiling existing software with newer kernel headers.
New kernel headers will supply updated ioctl numbers and kernel will
expect that userspace will use struct input_keymap_entry to set and
retrieve keymap data. But since the names of ioctls are still the same
userspace will happily compile even if not adjusted to make use of the
new structure and will start miraculously fail in the field.
To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
select the style of ioctls it wants to employ.
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/evdev.c | 113 +++++++++++++++++++++++++------------------------
include/linux/input.h | 6 ++-
2 files changed, 62 insertions(+), 57 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 17660b1..915287e 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -535,76 +535,73 @@ static int handle_eviocgbit(struct input_dev *dev,
}
#undef OLD_KEY_MAX
-static int evdev_handle_get_keycode(struct input_dev *dev,
- void __user *p, size_t size)
+static int evdev_handle_get_keycode(struct input_dev *dev, void __user *p)
{
- struct input_keymap_entry ke;
+ struct input_keymap_entry ke = {
+ .len = sizeof(unsigned int),
+ .flags = 0,
+ };
+ int __user *ip = (int __user *)p;
int error;
- memset(&ke, 0, sizeof(ke));
-
- if (size == sizeof(unsigned int[2])) {
- /* legacy case */
- int __user *ip = (int __user *)p;
+ /* legacy case */
+ if (copy_from_user(ke.scancode, p, sizeof(unsigned int)))
+ return -EFAULT;
- if (copy_from_user(ke.scancode, p, sizeof(unsigned int)))
- return -EFAULT;
+ error = input_get_keycode(dev, &ke);
+ if (error)
+ return error;
- ke.len = sizeof(unsigned int);
- ke.flags = 0;
+ if (put_user(ke.keycode, ip + 1))
+ return -EFAULT;
- error = input_get_keycode(dev, &ke);
- if (error)
- return error;
+ return 0;
+}
- if (put_user(ke.keycode, ip + 1))
- return -EFAULT;
+static int evdev_handle_get_keycode_v2(struct input_dev *dev, void __user *p)
+{
+ struct input_keymap_entry ke;
+ int error;
- } else {
- size = min(size, sizeof(ke));
+ if (copy_from_user(&ke, p, sizeof(ke)))
+ return -EFAULT;
- if (copy_from_user(&ke, p, size))
- return -EFAULT;
+ error = input_get_keycode(dev, &ke);
+ if (error)
+ return error;
- error = input_get_keycode(dev, &ke);
- if (error)
- return error;
+ if (copy_to_user(p, &ke, sizeof(ke)))
+ return -EFAULT;
- if (copy_to_user(p, &ke, size))
- return -EFAULT;
- }
return 0;
}
-static int evdev_handle_set_keycode(struct input_dev *dev,
- void __user *p, size_t size)
+static int evdev_handle_set_keycode(struct input_dev *dev, void __user *p)
{
- struct input_keymap_entry ke;
-
- memset(&ke, 0, sizeof(ke));
+ struct input_keymap_entry ke = {
+ .len = sizeof(unsigned int),
+ .flags = 0,
+ };
+ int __user *ip = (int __user *)p;
- if (size == sizeof(unsigned int[2])) {
- /* legacy case */
- int __user *ip = (int __user *)p;
+ if (copy_from_user(ke.scancode, p, sizeof(unsigned int)))
+ return -EFAULT;
- if (copy_from_user(ke.scancode, p, sizeof(unsigned int)))
- return -EFAULT;
+ if (get_user(ke.keycode, ip + 1))
+ return -EFAULT;
- if (get_user(ke.keycode, ip + 1))
- return -EFAULT;
+ return input_set_keycode(dev, &ke);
+}
- ke.len = sizeof(unsigned int);
- ke.flags = 0;
+static int evdev_handle_set_keycode_v2(struct input_dev *dev, void __user *p)
+{
+ struct input_keymap_entry ke;
- } else {
- size = min(size, sizeof(ke));
+ if (copy_from_user(&ke, p, sizeof(ke)))
+ return -EFAULT;
- if (copy_from_user(&ke, p, size))
- return -EFAULT;
-
- if (ke.len > sizeof(ke.scancode))
- return -EINVAL;
- }
+ if (ke.len > sizeof(ke.scancode))
+ return -EINVAL;
return input_set_keycode(dev, &ke);
}
@@ -670,6 +667,18 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
return evdev_grab(evdev, client);
else
return evdev_ungrab(evdev, client);
+
+ case EVIOCGKEYCODE:
+ return evdev_handle_get_keycode(dev, p);
+
+ case EVIOCSKEYCODE:
+ return evdev_handle_set_keycode(dev, p);
+
+ case EVIOCGKEYCODE_V2:
+ return evdev_handle_get_keycode_v2(dev, p);
+
+ case EVIOCSKEYCODE_V2:
+ return evdev_handle_set_keycode_v2(dev, p);
}
size = _IOC_SIZE(cmd);
@@ -709,12 +718,6 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
return -EFAULT;
return error;
-
- case EVIOC_MASK_SIZE(EVIOCGKEYCODE):
- return evdev_handle_get_keycode(dev, p, size);
-
- case EVIOC_MASK_SIZE(EVIOCSKEYCODE):
- return evdev_handle_set_keycode(dev, p, size);
}
/* Multi-number variable-length handlers */
diff --git a/include/linux/input.h b/include/linux/input.h
index 5e92384..ddd719d 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -104,8 +104,10 @@ struct input_keymap_entry {
#define EVIOCGREP _IOR('E', 0x03, unsigned int[2]) /* get repeat settings */
#define EVIOCSREP _IOW('E', 0x03, unsigned int[2]) /* set repeat settings */
-#define EVIOCGKEYCODE _IOR('E', 0x04, struct input_keymap_entry) /* get keycode */
-#define EVIOCSKEYCODE _IOW('E', 0x04, struct input_keymap_entry) /* set keycode */
+#define EVIOCGKEYCODE _IOR('E', 0x04, unsigned int[2]) /* get keycode */
+#define EVIOCGKEYCODE_V2 _IOR('E', 0x04, struct input_keymap_entry)
+#define EVIOCSKEYCODE _IOW('E', 0x04, unsigned int[2]) /* set keycode */
+#define EVIOCSKEYCODE_V2 _IOW('E', 0x04, struct input_keymap_entry)
#define EVIOCGNAME(len) _IOC(_IOC_READ, 'E', 0x06, len) /* get device name */
#define EVIOCGPHYS(len) _IOC(_IOC_READ, 'E', 0x07, len) /* get physical location */
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
2010-12-09 9:39 [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 Dmitry Torokhov
@ 2010-12-09 19:04 ` Henrik Rydberg
2010-12-09 19:16 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Henrik Rydberg @ 2010-12-09 19:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, LKML, linux-media, Mauro Carvalho Chehab,
Jiri Kosina, Jarod Wilson, David Härdeman
On 12/09/2010 10:39 AM, Dmitry Torokhov wrote:
> The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
> extending them to support large scancodes was a mistake. While we tried
> to keep ABI intact (and we succeeded in doing that, programs compiled
> on older kernels will work on newer ones) there is still a problem with
> recompiling existing software with newer kernel headers.
>
> New kernel headers will supply updated ioctl numbers and kernel will
> expect that userspace will use struct input_keymap_entry to set and
> retrieve keymap data. But since the names of ioctls are still the same
> userspace will happily compile even if not adjusted to make use of the
> new structure and will start miraculously fail in the field.
>
> To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
> and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
> select the style of ioctls it wants to employ.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
Would the header change suffice in itself? Either way, also checked that the
bugfixes following the original patch is still in effect, so looks ok to me.
Henrik
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
2010-12-09 19:04 ` Henrik Rydberg
@ 2010-12-09 19:16 ` Dmitry Torokhov
2010-12-13 9:06 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2010-12-09 19:16 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Linux Input, LKML, linux-media, Mauro Carvalho Chehab,
Jiri Kosina, Jarod Wilson, David Härdeman
On Thu, Dec 09, 2010 at 08:04:36PM +0100, Henrik Rydberg wrote:
> On 12/09/2010 10:39 AM, Dmitry Torokhov wrote:
>
> > The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
> > extending them to support large scancodes was a mistake. While we tried
> > to keep ABI intact (and we succeeded in doing that, programs compiled
> > on older kernels will work on newer ones) there is still a problem with
> > recompiling existing software with newer kernel headers.
> >
> > New kernel headers will supply updated ioctl numbers and kernel will
> > expect that userspace will use struct input_keymap_entry to set and
> > retrieve keymap data. But since the names of ioctls are still the same
> > userspace will happily compile even if not adjusted to make use of the
> > new structure and will start miraculously fail in the field.
> >
> > To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
> > and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
> > select the style of ioctls it wants to employ.
> >
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > ---
>
>
> Would the header change suffice in itself?
We still need to change evdev to return -EINVAL on wrong sizes but yes,
the amount of change there could be more limited. I just thought that
splitting it up explicitly shows the differences in handling better. If
people prefer the previos version we could leave it, I am 50/50 between
them.
> Either way, also checked that the
> bugfixes following the original patch is still in effect, so looks ok to me.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
2010-12-09 19:16 ` Dmitry Torokhov
@ 2010-12-13 9:06 ` Dmitry Torokhov
2010-12-13 18:31 ` Jarod Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2010-12-13 9:06 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Linux Input, LKML, linux-media, Mauro Carvalho Chehab,
Jiri Kosina, Jarod Wilson, David Härdeman
On Thu, Dec 09, 2010 at 11:16:47AM -0800, Dmitry Torokhov wrote:
> On Thu, Dec 09, 2010 at 08:04:36PM +0100, Henrik Rydberg wrote:
> > On 12/09/2010 10:39 AM, Dmitry Torokhov wrote:
> >
> > > The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
> > > extending them to support large scancodes was a mistake. While we tried
> > > to keep ABI intact (and we succeeded in doing that, programs compiled
> > > on older kernels will work on newer ones) there is still a problem with
> > > recompiling existing software with newer kernel headers.
> > >
> > > New kernel headers will supply updated ioctl numbers and kernel will
> > > expect that userspace will use struct input_keymap_entry to set and
> > > retrieve keymap data. But since the names of ioctls are still the same
> > > userspace will happily compile even if not adjusted to make use of the
> > > new structure and will start miraculously fail in the field.
> > >
> > > To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
> > > and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
> > > select the style of ioctls it wants to employ.
> > >
> > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > > ---
> >
> >
> > Would the header change suffice in itself?
>
> We still need to change evdev to return -EINVAL on wrong sizes but yes,
> the amount of change there could be more limited. I just thought that
> splitting it up explicitly shows the differences in handling better. If
> people prefer the previos version we could leave it, I am 50/50 between
> them.
>
*ping*
Mauro, Jarod, do you have an opinion on this? I think we need to settle
on a solution before 2.6.37 is out.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
2010-12-13 9:06 ` Dmitry Torokhov
@ 2010-12-13 18:31 ` Jarod Wilson
2010-12-14 1:54 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 6+ messages in thread
From: Jarod Wilson @ 2010-12-13 18:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Henrik Rydberg, Linux Input, LKML, linux-media,
Mauro Carvalho Chehab, Jiri Kosina, David Härdeman
On Mon, Dec 13, 2010 at 01:06:00AM -0800, Dmitry Torokhov wrote:
> On Thu, Dec 09, 2010 at 11:16:47AM -0800, Dmitry Torokhov wrote:
> > On Thu, Dec 09, 2010 at 08:04:36PM +0100, Henrik Rydberg wrote:
> > > On 12/09/2010 10:39 AM, Dmitry Torokhov wrote:
> > >
> > > > The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
> > > > extending them to support large scancodes was a mistake. While we tried
> > > > to keep ABI intact (and we succeeded in doing that, programs compiled
> > > > on older kernels will work on newer ones) there is still a problem with
> > > > recompiling existing software with newer kernel headers.
> > > >
> > > > New kernel headers will supply updated ioctl numbers and kernel will
> > > > expect that userspace will use struct input_keymap_entry to set and
> > > > retrieve keymap data. But since the names of ioctls are still the same
> > > > userspace will happily compile even if not adjusted to make use of the
> > > > new structure and will start miraculously fail in the field.
> > > >
> > > > To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
> > > > and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
> > > > select the style of ioctls it wants to employ.
> > > >
> > > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > > > ---
> > >
> > >
> > > Would the header change suffice in itself?
> >
> > We still need to change evdev to return -EINVAL on wrong sizes but yes,
> > the amount of change there could be more limited. I just thought that
> > splitting it up explicitly shows the differences in handling better. If
> > people prefer the previos version we could leave it, I am 50/50 between
> > them.
> >
>
> *ping*
>
> Mauro, Jarod, do you have an opinion on this? I think we need to settle
> on a solution before 2.6.37 is out.
Sorry, been meaning to reply, just been quite tied up with other work...
I'm of two minds on this as well, but probably leaning slightly in favor
of going ahead with an explicit _V2 so as to not break existing userspace
in new and unexpected ways. There presumably isn't much in the way of
userspace already adapted to the new interface, and its simple to do
another rev of those that have been. Okay, yeah, this is probably the best
way to go about it.
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2
2010-12-13 18:31 ` Jarod Wilson
@ 2010-12-14 1:54 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2010-12-14 1:54 UTC (permalink / raw)
To: Jarod Wilson
Cc: Dmitry Torokhov, Henrik Rydberg, Linux Input, LKML, linux-media,
Jiri Kosina, David Härdeman
Em 13-12-2010 16:31, Jarod Wilson escreveu:
> On Mon, Dec 13, 2010 at 01:06:00AM -0800, Dmitry Torokhov wrote:
>> On Thu, Dec 09, 2010 at 11:16:47AM -0800, Dmitry Torokhov wrote:
>>> On Thu, Dec 09, 2010 at 08:04:36PM +0100, Henrik Rydberg wrote:
>>>> On 12/09/2010 10:39 AM, Dmitry Torokhov wrote:
>>>>
>>>>> The desire to keep old names for the EVIOCGKEYCODE/EVIOCSKEYCODE while
>>>>> extending them to support large scancodes was a mistake. While we tried
>>>>> to keep ABI intact (and we succeeded in doing that, programs compiled
>>>>> on older kernels will work on newer ones) there is still a problem with
>>>>> recompiling existing software with newer kernel headers.
>>>>>
>>>>> New kernel headers will supply updated ioctl numbers and kernel will
>>>>> expect that userspace will use struct input_keymap_entry to set and
>>>>> retrieve keymap data. But since the names of ioctls are still the same
>>>>> userspace will happily compile even if not adjusted to make use of the
>>>>> new structure and will start miraculously fail in the field.
>>>>>
>>>>> To avoid this issue let's revert EVIOCGKEYCODE/EVIOCSKEYCODE definitions
>>>>> and add EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 so that userspace can explicitly
>>>>> select the style of ioctls it wants to employ.
>>>>>
>>>>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>>>>> ---
>>>>
>>>>
>>>> Would the header change suffice in itself?
>>>
>>> We still need to change evdev to return -EINVAL on wrong sizes but yes,
>>> the amount of change there could be more limited. I just thought that
>>> splitting it up explicitly shows the differences in handling better. If
>>> people prefer the previos version we could leave it, I am 50/50 between
>>> them.
>>>
>>
>> *ping*
>>
>> Mauro, Jarod, do you have an opinion on this? I think we need to settle
>> on a solution before 2.6.37 is out.
>
> Sorry, been meaning to reply, just been quite tied up with other work...
> I'm of two minds on this as well, but probably leaning slightly in favor
> of going ahead with an explicit _V2 so as to not break existing userspace
> in new and unexpected ways. There presumably isn't much in the way of
> userspace already adapted to the new interface, and its simple to do
> another rev of those that have been. Okay, yeah, this is probably the best
> way to go about it.
>
> Acked-by: Jarod Wilson <jarod@redhat.com>
>
>
I'm with some email troubles here. Not sure if you got my answer. I'm OK with
those changes.
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com
Cheers,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-14 1:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-09 9:39 [RFC] Input: define separate EVIOCGKEYCODE_V2/EVIOCSKEYCODE_V2 Dmitry Torokhov
2010-12-09 19:04 ` Henrik Rydberg
2010-12-09 19:16 ` Dmitry Torokhov
2010-12-13 9:06 ` Dmitry Torokhov
2010-12-13 18:31 ` Jarod Wilson
2010-12-14 1:54 ` Mauro Carvalho Chehab
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).