[PATCH libevdev 4/4] Transparently use EVIOCSMASK to disable event codes
David Herrmann
dh.herrmann at gmail.com
Fri Apr 22 08:58:16 UTC 2016
Hi
On Thu, Apr 21, 2016 at 11:59 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> Disable the event code at the kernel level. When set, the kernel doesn't pass
> that code on to userspace at all so we reduce the wakeups needed.
>
> Rather than setting the mask immediately on disabling/enabling, we wait until
> the next libevdev_next_event() call and update the mask then. This reduces
> ioctls if multiple event codes need to be disabled or enabled.
>
> This introduces a behavior change: previously, libevdev would update the
> internal device state even for disabled event codes. Even though the event
> wouldn't reach the client, a call to libevdev_get_event_value() would yield
> the current kernel state. With this state libevdev itself does not see the
> events and cannot update its state. libevdev_get_event_value() will thus
> return the last known state only. The use-case of disabling a code but relying
> on libevdev's state is discouraged though.
I *really* like the idea of using EVIOCSMASK transparently in
libevdev. Especially given the awkward overflow behavior, it should
really be hidden in the library. Anyway, I think the main use-case for
masking event-codes is a whitelist rather than a blacklist. So clients
want to do things like: give me nothing but KEY_POWER. With the
current libevdev API this is a bit awkward, as you need to iterate all
KEYs, disable each one that *might* be present, ... I guess you see my
point.
Anyway, this is probably unrelated to this patch, as a simple addition
of something like libevdev_disable_everything() would be enough.
However, what is the reason you don't allow masking entire TYPEs? So
we already have libevdev_disable_event_type(), along
libevdev_disable_event_code(). EVIOCSMASK allows disabling entire
types by using EV_SYN (special behavior, just like for EVIOCGBIT).
Right now, if you call libevdev_disable_event_type(), it will not make
use of EVIOCSMASK.
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> libevdev/libevdev-int.h | 6 ++++++
> libevdev/libevdev.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> libevdev/libevdev.h | 5 +++++
> 3 files changed, 60 insertions(+)
>
> diff --git a/libevdev/libevdev-int.h b/libevdev/libevdev-int.h
> index 6c5b8b6..bced2e2 100644
> --- a/libevdev/libevdev-int.h
> +++ b/libevdev/libevdev-int.h
> @@ -96,6 +96,12 @@ struct libevdev {
> unsigned long key_values[NLONGS(KEY_CNT)];
> unsigned long led_values[NLONGS(LED_CNT)];
> unsigned long sw_values[NLONGS(SW_CNT)];
> + /* A bit for each event type, if set we need to update the
> + * EVIOCSMASK. EV_SYN is used as general flag that another one has
> + * changed
> + */
> + unsigned long mask_update_needed[NLONGS(EV_CNT)];
> +
> struct input_absinfo abs_info[ABS_CNT];
> int *mt_slot_vals; /* [num_slots * ABS_MT_CNT] */
> int num_slots; /**< valid slots in mt_slot_vals */
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index 149a0d7..194b17c 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -24,6 +24,7 @@
> #include <errno.h>
> #include <poll.h>
> #include <stdlib.h>
> +#include <stdint.h>
> #include <string.h>
> #include <limits.h>
> #include <unistd.h>
> @@ -1007,6 +1008,43 @@ sanitize_event(const struct libevdev *dev,
> return EVENT_FILTER_NONE;
> }
>
> +static inline int
> +update_event_mask(struct libevdev *dev)
> +{
> + int rc = 0;
> + int type;
> +
> + /* skip EV_SYN, it's the general "update needed" flag */
> + for (type = 1; type < EV_CNT; type++) {
> + unsigned long *m;
> + int max;
> + size_t sz;
> + struct input_mask mask;
> +
> + if (!bit_is_set(dev->mask_update_needed, type))
> + continue;
> +
> + max = type_to_mask(dev, type, &m, &sz);
> + if (max == -1)
> + continue;
> +
> + mask.type = type;
> + mask.codes_size = sz;
> + mask.codes_ptr = (uint64_t)m;
> + rc = ioctl(dev->fd, EVIOCSMASK, &mask);
> + if (rc == -1) {
> + if (errno == EINVAL) /* unsupported by kernel */
> + rc = 0;
> + goto out;
So we don't cache whether EVIOCSMASK is supported. Probably fine,
since you're not supposed to change your masks at runtime at all, so
we call it only once anyway. Just wanted to know whether that is
intentional?
Otherwise, I really like it! Thanks!
David
> + }
> + }
> +
> +out:
> + memset(dev->mask_update_needed, 0, sizeof(dev->mask_update_needed));
> +
> + return rc == -1 ? -errno : 0;
> +}
> +
> LIBEVDEV_EXPORT int
> libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event *ev)
> {
> @@ -1028,6 +1066,9 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
> return -EINVAL;
> }
>
> + if (bit_is_set(dev->mask_update_needed, EV_SYN))
> + update_event_mask(dev);
> +
> if (flags & LIBEVDEV_READ_FLAG_SYNC) {
> if (dev->sync_state == SYNC_NEEDED) {
> rc = sync_state(dev);
> @@ -1485,6 +1526,10 @@ libevdev_enable_event_code(struct libevdev *dev, unsigned int type,
> return -1;
>
> set_bit(mask, code);
> + if (type != EV_SYN) {
> + set_bit(dev->mask_update_needed, type);
> + set_bit(dev->mask_update_needed, EV_SYN);
> + }
>
> if (type == EV_ABS) {
> const struct input_absinfo *abs = data;
> @@ -1512,6 +1557,10 @@ libevdev_disable_event_code(struct libevdev *dev, unsigned int type, unsigned in
> return -1;
>
> clear_bit(mask, code);
> + if (type != EV_SYN) {
> + set_bit(dev->mask_update_needed, type);
> + set_bit(dev->mask_update_needed, EV_SYN);
> + }
>
> return 0;
> }
> diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
> index 856aeb9..a12a8d7 100644
> --- a/libevdev/libevdev.h
> +++ b/libevdev/libevdev.h
> @@ -528,6 +528,8 @@ extern "C" {
> * <dt>EVIOCREVOKE:</dt>
> * <dd>currently not supported, see
> * http://lists.freedesktop.org/archives/input-tools/2014-January/000688.html</dd>
> + * <dt>EVIOCGMASK/EVIOCGSMASK:</dt>
> + * <dd>supported, see libevdev_disable_event_code()
> * </dl>
> *
> */
> @@ -1858,6 +1860,9 @@ int libevdev_enable_event_code(struct libevdev *dev, unsigned int type, unsigned
> * Disabling codes of type EV_SYN will not work. Don't shoot yourself in the
> * foot. It hurts.
> *
> + * libevdev may transparently use the EVIOCSMASK ioctl if available. This
> + * ioctl disables delivery of that event type in the kernel.
> + *
> * @note This function should only be invoked when the respective event type
> * is in a neutral state. Otherwise, a caller may get unexpected event
> * sequences. For example, disabling an event code while a touch is down may
> --
> 2.7.3
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/input-tools
More information about the Input-tools
mailing list