Em 25-01-2011 18:54, Dmitry Torokhov escreveu:
On Wed, Jan 26, 2011 at 06:09:45AM +1000, Linus Torvalds wrote:
quoted
On Wed, Jan 26, 2011 at 2:48 AM, Dmitry Torokhov
[off-list ref] wrote:
quoted
We should be able to handle the case where scancode is valid even though
it might be unmapped yet. This is regardless of what version of
EVIOCGKEYCODE we use, 1 or 2, and whether it is sparse keymap or not.
Is it possible to validate the scancode by driver?
More appropriately, why not just revert the thing? The version change
Reverting the version increment is a bad thing. I agree with Dmitry that
an application that fails just because the API version were incremented
is buggy.
Well, then we'll break Ubuntu again as they recompiled their input-utils
package (without fixing the check). And the rest of distros do not seem
to be using that package...
Reverting it will also break the ir-keytable userspace program that it is
meant to be used by the Remote Controller devices, and uses it to adjust
its behaviour to support RC's with more than 16 bits of scancodes.
I agree that it is bad that the ABI broke, but reverting it will cause even
more damage.
quoted
and the buggy EINVAL return both.
I believe that -EINVAL thing only affects RC devices that Mauro switched
to the new rc-core; input core in itself should be ABI compatible. Thus
I'll leave the decision to him whether he wants to revert or fix
compatibility issue.
The Remote Controller keycode tables are very sparse. In general,
they contain up to 100 entries, and the scan codes typically have 16
bits. Some newer devices have 24 or 32 bits. With version 1, as the table
index is the scancode, in order to read all keytables with EVIOCGKEYCODE,
the userspace needs to do 2^16 reads (or 2^32 for RC-6 remotes).
I don't need to say that this is highly ineffective. So, using V1
doesn't work fine anyway for Remote Controllers.
Btw, ir-keycodestool don't work with V1 and more than 16 bits, because it
doesn't scale. I didn't actually checked, but based on Dmitry's patch
for input-kbd, it is clear to me that the old version only supports 16
bits scancodes:
Em 25-01-2011 04:52, Dmitry Torokhov escreveu:
quoted hunk
From c22c85c0b675422a23e3d853ed06fedc36805774 Mon Sep 17 00:00:00 2001
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Mon, 24 Jan 2011 22:49:59 -0800
Subject: [PATCH] input-kbd - switch to using EVIOCGKEYCODE2 when available
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
input-kbd.c | 118 ++++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 80 insertions(+), 38 deletions(-)
diff --git a/input-kbd.c b/input-kbd.c
index e94529d..5d93d54 100644
--- a/input-kbd.c
+++ b/input-kbd.c
<snip>
quoted hunk
@@ -23,7 +41,7 @@ struct kbd_map {
/* ------------------------------------------------------------------ */
-static struct kbd_map* kbd_map_read(int fd)
+static struct kbd_map* kbd_map_read(int fd, unsigned int version)
{
struct kbd_entry entry;
struct kbd_map *map;@@ -32,16 +50,37 @@ static struct kbd_map* kbd_map_read(int fd)
map = malloc(sizeof(*map));
memset(map,0,sizeof(*map));
for (map->size = 0; map->size < 65536; map->size++) {
See, it will only look into the 16-bits scancode space. There are several remote
controllers with 24 bits and 32 bits, so the tool is already broken anyway.
On the tests I did here with an ir-keytable version made before such change,
with a Fedora rawhide kernel (2.6.37), I didn't notice any breakage at
EVIOCGKEYCODE. I'll do more tests tomorrow with a vanilla Kernel. I'll
compile a vanilla 2.6.37 kernel tomorrow and, if needed, write a patch.
quoted
As Mark said, breaking user space simply isn't acceptable. And since
breaking user space isn't acceptable, then incrementing the version is
stupid too.
It might not have been the best idea to increment, however I maintain
that if there exists version is can be changed. Otherwise there is no
point in having version at all.
Not arguing in favor of the version numbering, but it is easy to read
the version increment at the beginning of the application, and adjust
if the code will use EVIOCGKEYCODE or EVIOCGKEYCODE_V2 of the ioctl's,
depending on what kernel provides.
Ok, we might be just calling the new ioctl and check for -ENOSYS at
the beginning, using some fake arguments.
As I said, reverting the version bump will cause yet another wave of
breakages so I propose leaving version as is.
quoted
The way we add new ioctl's is not by incrementing some "ABI version"
crap. It's by adding new ioctl's or system calls or whatever that
simply used to return -ENOSYS or other error before, while preserving
the old ABI. That way old binaries don't break (for _ANY_ reason), and
new binaries can see "oh, this doesn't support the new thing".
That has been done as well; we have 2 new ioctls and kept 2 old ioctls.
Regards,
Mauro.