ff-core effect id handling in case of a failed effect upload

[Elias Vanderstuyft]

On Wed, Feb 19, 2014 at 10:05 PM, Dmitry Torokhov
<dmitry.torokhov at gmail.com> wrote:
> On Wed, Feb 19, 2014 at 06:49:36PM +0200, Anssi Hannula wrote:
>> (added Dmitry to CC)
>>
>> 19.02.2014 13:42, Elias Vanderstuyft kirjoitti:
>> > Hi,
>> >
>>
>> Hi,
>>
>> > In the process of reviewing the Wine DInput translation layer, I
>> > noticed an inconvenience (in the ff-core implementation?) that can
>> > possibly lead to confusing problems to application developers (not
>> > only for Wine), in short:
>> > If a new (id==-1) effect was uploaded (look at
>> > ff-core.c::input_ff_upload(...)) that failed (e.g. returning EINVAL),
>> > ff-core will have assigned a positive number to the effect id. This
>> > can be confusing because the dev->ff->effects[] array will not contain
>> > an element at the index of that new effect id.
>>
>> I agree that this is a bit confusing, and the kernel code should
>> probably be modified to not clobber the ioctl-provided effect on failure
>> (effect->id is set to an "undefined" value, i.e. next free effect slot).
>>
>> Dmitry, WDYT?
>
> Yeah, it looks like we need to change evdev.c to read:
>
>                 error = input_ff_upload(dev, &effect, file);
>                 if (error)
>                         return error;
>
>                 if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
>                         return -EFAULT;
>
>                 return 0;

Alright, who will create the patch?
Do I may / have to do it?

>
> Unfortunately applications should still expect changed effect ID for
> quite some time.
>
> Thanks.
>
> --
> Dmitry

Best regards,
Elias