Hi On Tue, Apr 22, 2014 at 6:29 AM, Peter Hutterer <peter.hutterer@xxxxxxxxx> wrote: >> +/* requires the buffer lock to be held */ >> +static bool __evdev_is_masked(struct evdev_client *client, >> + unsigned int type, >> + unsigned int code) >> +{ >> + unsigned long *mask; >> + size_t cnt; >> + >> + /* EV_SYN and unknown codes are never masked */ >> + if (!type || type >= EV_CNT) > > why not use type == EV_SYN? You mean instead of "!type"? Yeah, probably easier to read. >> + return false; >> + >> + /* first test whether the type is masked */ >> + mask = client->evmasks[0]; > > if mask is NULL, you already know it's not mask, you can return early. Nope. If evmasks[0] is NULL, then no type-mask has been set. That doesn't mean the given code-mask is NULL. For instance: evmasks[0] == NULL evmasks[EV_KEY][KEY_A] == 0 In that case, the user wants all events but KEY_A (even though evmasks[0] is NULL). >> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */ >> #define EVIOCREVOKE _IOW('E', 0x91, int) /* Revoke device access */ >> +#define EVIOCGMASK _IOR('E', 0x92, struct input_mask) /* Get event-masks */ >> +#define EVIOCSMASK _IOW('E', 0x93, struct input_mask) /* Set event-masks */ > > This is missing from all other ioctls but while you're adding a new one > anyway: please add documentation on what the ioctl does, the input and > return value/output expected, side-effects etc. right now, understanding the > evdev ioctls requires either reading the kernel code or existing user-space > code, with the usual risk of getting it wrong. Meh.. I thought no-one will notice.. Clearly, I was wrong. I will add proper docs in v2. Thanks a lot! 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