uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required

uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required

[John Salamon]

Currently the "fake" input events generated by uinput in response to
effect uploads will return an effect with an id that has already been
handled by input_ff_upload in ff-core.c, which can modify the effect
id. This causes a problem specifically when the effect originally
uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
userspace code handling UI_FF_UPLOAD receives an effect with an id
other than -1, and therefore will not know an allocation was
requested.

I notice that the "old" field on the ff_effect struct is set to NULL
when the -1 id is changed (in input_ff_upload), which can serve as a
flag that an allocation was requested. If it is the intention is that
uinput users check if old == NULL to know when allocations are needed
I think uinput documentation should describe this.

I first noticed this using python-evdev, see my issue report here:
https://github.com/gvalkov/python-evdev/issues/199

Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required

[John Salamon]

Correction, "old" is a pointer to struct ff_effect, which after being
set to NULL looks like it gets pushed out by uinput on a struct
uinput_request.


On Tue, Oct 10, 2023 at 5:38 PM John Salamon <salamonj9@gmail.com> wrote:
>
> Currently the "fake" input events generated by uinput in response to
> effect uploads will return an effect with an id that has already been
> handled by input_ff_upload in ff-core.c, which can modify the effect
> id. This causes a problem specifically when the effect originally
> uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> userspace code handling UI_FF_UPLOAD receives an effect with an id
> other than -1, and therefore will not know an allocation was
> requested.
>
> I notice that the "old" field on the ff_effect struct is set to NULL
> when the -1 id is changed (in input_ff_upload), which can serve as a
> flag that an allocation was requested. If it is the intention is that
> uinput users check if old == NULL to know when allocations are needed
> I think uinput documentation should describe this.
>
> I first noticed this using python-evdev, see my issue report here:
> https://github.com/gvalkov/python-evdev/issues/199

Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required

[Dmitry Torokhov]

Hi John,

On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> Currently the "fake" input events generated by uinput in response to
> effect uploads will return an effect with an id that has already been
> handled by input_ff_upload in ff-core.c, which can modify the effect
> id. This causes a problem specifically when the effect originally
> uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> userspace code handling UI_FF_UPLOAD receives an effect with an id
> other than -1, and therefore will not know an allocation was
> requested.

The kernel never changes ID of an existing effect, the only time ID is
changed is when userspace indicates that a new effect should be created
by setting effect ID to -1.

The handler of force feedback effects should know what effects (with
what IDs) have been uploaded to the device so far, so whenever it sees a
request for an effect with previously unseen effect_id it should
recognize this as a signal that a new effect/id has been allocated by
the kernel.

> 
> I notice that the "old" field on the ff_effect struct is set to NULL
> when the -1 id is changed (in input_ff_upload), which can serve as a
> flag that an allocation was requested. If it is the intention is that
> uinput users check if old == NULL to know when allocations are needed
> I think uinput documentation should describe this.

No, not really, as explained above.

> 
> I first noticed this using python-evdev, see my issue report here:
> https://github.com/gvalkov/python-evdev/issues/199

Thanks.

-- 
Dmitry

Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required

[John Salamon]

Hi Dmitry,

Thank you for the clarification.

> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.

In that case, would you consider accepting a patch adding a sentence
or two explaining that handlers will need to keep track of effect IDs
in this manner? Either in uinput documentation or just in the comment
describing how to implement upload_effect() in uinput.h. Something
along the lines of "Effects received here will never have an ID of -1.
Handlers of these effects must recognize previously unseen effect IDs
to know when a new effect has been allocated.".

Cheers,
John


On Sat, Oct 14, 2023 at 8:29 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi John,
>
> On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> > Currently the "fake" input events generated by uinput in response to
> > effect uploads will return an effect with an id that has already been
> > handled by input_ff_upload in ff-core.c, which can modify the effect
> > id. This causes a problem specifically when the effect originally
> > uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> > userspace code handling UI_FF_UPLOAD receives an effect with an id
> > other than -1, and therefore will not know an allocation was
> > requested.
>
> The kernel never changes ID of an existing effect, the only time ID is
> changed is when userspace indicates that a new effect should be created
> by setting effect ID to -1.
>
> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.
>
> >
> > I notice that the "old" field on the ff_effect struct is set to NULL
> > when the -1 id is changed (in input_ff_upload), which can serve as a
> > flag that an allocation was requested. If it is the intention is that
> > uinput users check if old == NULL to know when allocations are needed
> > I think uinput documentation should describe this.
>
> No, not really, as explained above.
>
> >
> > I first noticed this using python-evdev, see my issue report here:
> > https://github.com/gvalkov/python-evdev/issues/199
>
> Thanks.
>
> --
> Dmitry