Hi
On Mon, Jul 21, 2014 at 3:01 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi David,
>
> On Sat, Jul 19, 2014 at 03:10:44PM +0200, David Herrmann wrote:
>> This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
>> method (by write()'ing "struct uinput_user_dev" to the node). The old
>> method is not easily extendable and requires huge payloads. Furthermore,
>> overloading write() without properly versioned objects is error-prone.
>>
>> Therefore, we introduce a new ioctl to replace the old method. The ioctl
>> supports all features of the old method, plus a "resolution" field for
>> absinfo. Furthermore, it's properly forward-compatible to new ABS codes
>> and a growing "struct input_absinfo" structure.
>>
>> The ioctl also allows user-space to skip unknown axes if not set. The
>> payload-size can now be specified by the caller. There is no need to copy
>> the whole array temporarily into the kernel, but instead we can iterate
>> over it and copy each value manually.
>>
>> Reviewed-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
>> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
>> ---
>> drivers/input/misc/uinput.c | 69 +++++++++++++++++++++++++++++++++++++++++++--
>> include/uapi/linux/uinput.h | 55 ++++++++++++++++++++++++++++++++++--
>> 2 files changed, 118 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index a2a3895..0f45595 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -371,8 +371,67 @@ static int uinput_allocate_device(struct uinput_device *udev)
>> return 0;
>> }
>>
>> -static int uinput_setup_device(struct uinput_device *udev,
>> - const char __user *buffer, size_t count)
>> +static int uinput_dev_setup(struct uinput_device *udev,
>> + struct uinput_setup __user *arg)
>> +{
>> + struct uinput_setup setup;
>> + struct input_dev *dev;
>> + int i, retval;
>> +
>> + if (udev->state == UIST_CREATED)
>> + return -EINVAL;
>> + if (copy_from_user(&setup, arg, sizeof(setup)))
>> + return -EFAULT;
>> + if (!setup.name[0])
>> + return -EINVAL;
>> + /* So far we only support the original "struct input_absinfo", but be
>> + * forward compatible and allow larger payloads. */
>> + if (setup.absinfo_size < sizeof(struct input_absinfo))
>> + return -EINVAL;
>
> No, we can not do this, as it breaks backward compatibility (the most
> important one!). If we were to increase size of in-kernel input_absinfo
> in let's say 3.20, userspace compiled against older kernel headers
> (but using the new ioctl available let's say since 3.16 - don't hold me
> to the numbers ;) ), would break since it wold start tripping on thi
> check.
>
> The proper way to handle it is to convert "old" absinfo into new one,
> applying as much as we can.
I know, but there is no "old absinfo". Once we extend "struct absinfo"
I expect this code to change to:
{
/* initially supported absinfo had size 24 */
if (setup.absinfo_size < 24)
return -EINVAL;
/* ...pseudo code... */
memset(&absinfo, 0, sizeof(absinfo));
memcpy(&absinfo, sth->absinfo, MIN(setup.absinfo_size,
sizeof(absinfo)));
}
This allows you to use this ioctl with old absinfo objects. I can
change the current code to this already, if you want? I tried to avoid
it, because a memset() is not neccessarily an appropriate way to
initialize unset fields.
I cal also add support for "absinfo" without the "resolution" field,
which I think is the only field that wasn't available in the initial
structure.
Let me know which way you prefer.
Thanks
David
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
