diff mbox

[1/1] Input: don't modify the id of ioctl-provided ff effect on upload failure

Message ID CADbOyBTf4fWhUveA7UgCP4EpLXAyrZCV-xs-wZ7HGyBwxhQFhg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Elias Vanderstuyft Feb. 23, 2014, 2:18 p.m. UTC
From: Elias Vanderstuyft <elias.vds@gmail.com>

If a new (id == -1) ff effect was uploaded from userspace,
ff-core.c::input_ff_upload() will have assigned
a positive number to the new effect id.
Currently, evdev.c::evdev_do_ioctl() will save this new id to userspace,
regardless of whether the upload succeeded or not.

On upload failure, this can be confusing because the dev->ff->effects[] array
will not contain an element at the index of that new effect id.

Note: Unfortunately applications should still expect changed effect id for
quite some time.

This has been discussed on:
http://www.mail-archive.com/linux-input@vger.kernel.org/msg08513.html
("ff-core effect id handling in case of a failed effect upload")

Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Elias Vanderstuyft <elias.vds@gmail.com>
---
 drivers/input/evdev.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Elias Vanderstuyft March 15, 2014, 12:13 p.m. UTC | #1
On Sun, Feb 23, 2014 at 3:18 PM, Elias Vanderstuyft <elias.vds@gmail.com> wrote:
> From: Elias Vanderstuyft <elias.vds@gmail.com>
>
> If a new (id == -1) ff effect was uploaded from userspace,
> ff-core.c::input_ff_upload() will have assigned
> a positive number to the new effect id.
> Currently, evdev.c::evdev_do_ioctl() will save this new id to userspace,
> regardless of whether the upload succeeded or not.
>
> On upload failure, this can be confusing because the dev->ff->effects[] array
> will not contain an element at the index of that new effect id.
>
> Note: Unfortunately applications should still expect changed effect id for
> quite some time.
>
> This has been discussed on:
> http://www.mail-archive.com/linux-input@vger.kernel.org/msg08513.html
> ("ff-core effect id handling in case of a failed effect upload")
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Elias Vanderstuyft <elias.vds@gmail.com>
> ---
>  drivers/input/evdev.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/drivers/input/evdev.c     2014-02-23 14:21:19.980606615 +0100
> +++ b/drivers/input/evdev.c     2014-02-23 14:25:04.417118859 +0100
> @@ -954,11 +954,13 @@ static long evdev_do_ioctl(struct file *
>                         return -EFAULT;
>
>                 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 error;
> +               return 0;
>         }
>
>         /* Multi-number variable-length handlers */

This is a reminder that there have been no replies on this subject for
the last 3 weeks.
If there is something wrong with the format of this patch, please tell
me, I tried to follow the instructions on
http://www.kernel.org/doc/Documentation/SubmittingPatches , but it's
the first time I send a patch by myself, so I might have forgotten
something.
Did I forget to CC some people?

The reason I'm insisting on this, is because this change will be
perceivable to userspace developers, and the sooner this gets applied,
the less userland applications will have to circumvent this bug.


Best regards,

Elias
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- a/drivers/input/evdev.c     2014-02-23 14:21:19.980606615 +0100
+++ b/drivers/input/evdev.c     2014-02-23 14:25:04.417118859 +0100
@@ -954,11 +954,13 @@  static long evdev_do_ioctl(struct file *
                        return -EFAULT;

                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 error;
+               return 0;
        }

        /* Multi-number variable-length handlers */