Re: [PATCH v2 4/7] Input: simplify event handling logic

Jeff LaBundy
2024-07-07T19:55:28Z

Hi Dmitry,

On Wed, Jul 03, 2024 at 02:37:51PM -0700, Dmitry Torokhov wrote:
> Streamline event handling code by providing batch implementations for
> filtering and event processing and using them in place of the main
> event handler, as needed, instead of having complex branching logic
> in the middle of the event processing code.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Jeff LaBundy <jeff@labundy.com>

> ---
> drivers/input/input.c | 109 ++++++++++++++++++++++++++----------------
> 1 file changed, 68 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 7e4f8824f4fd..40a04154f99d 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -99,45 +99,13 @@ static void input_stop_autorepeat(struct input_dev *dev)
> del_timer(&dev->timer);
> }
>
> -/*
> - * Pass event first through all filters and then, if event has not been
> - * filtered out, through all open handles. This function is called with
> - * dev->event_lock held and interrupts disabled.
> - */
> -static unsigned int input_to_handler(struct input_handle *handle,
> - struct input_value *vals, unsigned int count)
> -{
> - struct input_handler *handler = handle->handler;
> - struct input_value *end = vals;
> - struct input_value *v;
> -
> - if (handler->filter) {
> - for (v = vals; v != vals + count; v++) {
> - if (handler->filter(handle, v->type, v->code, v->value))
> - continue;
> - if (end != v)
> - *end = *v;
> - end++;
> - }
> - count = end - vals;
> - }
> -
> - if (!count)
> - return 0;
> -
> - if (handler->events)
> - handler->events(handle, vals, count);
> - else if (handler->event)
> - for (v = vals; v != vals + count; v++)
> - handler->event(handle, v->type, v->code, v->value);
> -
> - return count;
> -}
> -
> /*
> * Pass values first through all filters and then, if event has not been
> - * filtered out, through all open handles. This function is called with
> - * dev->event_lock held and interrupts disabled.
> + * filtered out, through all open handles. This order is achieved by placing
> + * filters at the head of the list of handles attached to the device, and
> + * placing regular handles at the tail of the list.
> + *
> + * This function is called with dev->event_lock held and interrupts disabled.
> */
> static void input_pass_values(struct input_dev *dev,
> struct input_value *vals, unsigned int count)
> @@ -154,11 +122,12 @@ static void input_pass_values(struct input_dev *dev,
>
> handle = rcu_dereference(dev->grab);
> if (handle) {
> - count = input_to_handler(handle, vals, count);
> + count = handle->handler->events(handle, vals, count);
> } else {
> list_for_each_entry_rcu(handle, &dev->h_list, d_node)
> if (handle->open) {
> - count = input_to_handler(handle, vals, count);
> + count = handle->handler->events(handle, vals,
> + count);
> if (!count)
> break;
> }
> @@ -2537,6 +2506,57 @@ static int input_handler_check_methods(const struct input_handler *handler)
> return 0;
> }
>
> +/*
> + * An implementation of input_handler's events() method that simply
> + * invokes handler->event() method for each event one by one.
> + */
> +static unsigned int input_handler_events_default(struct input_handle *handle,
> + struct input_value *vals,
> + unsigned int count)
> +{
> + struct input_handler *handler = handle->handler;
> + struct input_value *v;
> +
> + for (v = vals; v != vals + count; v++)
> + handler->event(handle, v->type, v->code, v->value);
> +
> + return count;
> +}
> +
> +/*
> + * An implementation of input_handler's events() method that invokes
> + * handler->filter() method for each event one by one and removes events
> + * that were filtered out from the "vals" array.
> + */
> +static unsigned int input_handler_events_filter(struct input_handle *handle,
> + struct input_value *vals,
> + unsigned int count)
> +{
> + struct input_handler *handler = handle->handler;
> + struct input_value *end = vals;
> + struct input_value *v;
> +
> + for (v = vals; v != vals + count; v++) {
> + if (handler->filter(handle, v->type, v->code, v->value))
> + continue;
> + if (end != v)
> + *end = *v;
> + end++;
> + }
> +
> + return end - vals;
> +}
> +
> +/*
> + * An implementation of input_handler's events() method that does nothing.
> + */
> +static unsigned int input_handler_events_null(struct input_handle *handle,
> + struct input_value *vals,
> + unsigned int count)
> +{
> + return count;
> +}
> +
> /**
> * input_register_handler - register a new input handler
> * @handler: handler to be registered
> @@ -2554,12 +2574,19 @@ int input_register_handler(struct input_handler *handler)
> if (error)
> return error;
>
> + INIT_LIST_HEAD(&handler->h_list);
> +
> + if (handler->filter)
> + handler->events = input_handler_events_filter;
> + else if (handler->event)
> + handler->events = input_handler_events_default;
> + else if (!handler->events)
> + handler->events = input_handler_events_null;
> +
> error = mutex_lock_interruptible(&input_mutex);
> if (error)
> return error;
>
> - INIT_LIST_HEAD(&handler->h_list);
> -
> list_add_tail(&handler->node, &input_handler_list);
>
> list_for_each_entry(dev, &input_dev_list, node)
> --
> 2.45.2.803.g4e1b14247a-goog
>

Kind regards,
Jeff LaBundy