Hi
On Mon, Jul 21, 2014 at 10:11 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Mon, Jul 21, 2014 at 08:22:09AM +0200, David Herrmann wrote:
>> 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.
>
> I think for now I would do:
>
> /*
> * Whoever is changing struct input_absinfo will have to take
> * care of backwards compatibility.
> */
> BUILD_BUG_ON(sizeof(struct input_absinfo)) != 24);
> if (setup.absinfo_size != sizeof(struct input_absinfo))
> return -EINVAL;
>
> ...
>
> and later when we detect setup.absinfo_size < sizeof(struct
> input_absinfo) we'll have to take care about backwards compatibility. We
> do not need to take care of forward compatibility as we do not know if
> userspace will be satisfied with partial results or not and newer
> userspace can deal with proper handling of older kernels.
I'm not sure I agree. I'm a fan of forward-compatibility, but that's
probably a matter of taste. I will change my code accordingly.
> By the way, I realize I do not like the new IOCTL as it is - it's too
> big and would be hard to extend if we want to change items other than
> absinfo. Why don't we create UI_DEV_SETUP that only sets name, id, and
> number of effects, and add UI_SET_ABSAXIS that would take:
>
> struct uinput_abs_setup {
> __u16 code; /* axis code */
> /* __u16 filler; */
> struct input_absinfo absinfo;
> }
Hm, that is actually a good idea. I will give it a try.
> By the way, while you are hacking on uinput can we also fix formatting
> style of switch/case and switch printk() to pr_debug() and friends? I'd
> do it myself but do not want to step on your toes.
Yepp, I will include those in the next revision.
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
