Hi,
On 8/25/24 7:16 AM, Dmitry Torokhov wrote:
> Start using __free() and guard() primitives to simplify the code
> and error handling. This makes the code more compact and error
> handling more robust by ensuring that locks are released in all
> code paths when control leaves critical section and all allocated
> memory is freed.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/input/keyboard/gpio_keys.c | 44 ++++++++++++------------------
> 1 file changed, 17 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 9fb0bdcfbf9e..380fe8dab3b0 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -245,23 +245,20 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata,
> {
> int n_events = get_n_events_by_type(type);
> const unsigned long *bitmap = get_bm_events_by_type(ddata->input, type);
> - unsigned long *bits;
> ssize_t error;
> int i;
>
> - bits = bitmap_alloc(n_events, GFP_KERNEL);
> + unsigned long *bits __free(bitmap) = bitmap_alloc(n_events, GFP_KERNEL);
> if (!bits)
> return -ENOMEM;
>
> error = bitmap_parselist(buf, bits, n_events);
> if (error)
> - goto out;
> + return error;
>
> /* First validate */
> - if (!bitmap_subset(bits, bitmap, n_events)) {
> - error = -EINVAL;
> - goto out;
> - }
> + if (!bitmap_subset(bits, bitmap, n_events))
> + return -EINVAL;
>
> for (i = 0; i < ddata->pdata->nbuttons; i++) {
> struct gpio_button_data *bdata = &ddata->data[i];
> @@ -271,12 +268,11 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata,
>
> if (test_bit(*bdata->code, bits) &&
> !bdata->button->can_disable) {
> - error = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
> }
>
> - mutex_lock(&ddata->disable_lock);
> + guard(mutex)(&ddata->disable_lock);
>
> for (i = 0; i < ddata->pdata->nbuttons; i++) {
> struct gpio_button_data *bdata = &ddata->data[i];
> @@ -290,11 +286,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata,
> gpio_keys_enable_button(bdata);
> }
>
> - mutex_unlock(&ddata->disable_lock);
> -
> -out:
> - bitmap_free(bits);
> - return error;
> + return 0;
> }
>
> #define ATTR_SHOW_FN(name, type, only_disabled) \
> @@ -470,11 +462,10 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
> {
> struct gpio_button_data *bdata = dev_id;
> struct input_dev *input = bdata->input;
> - unsigned long flags;
>
> BUG_ON(irq != bdata->irq);
>
> - spin_lock_irqsave(&bdata->lock, flags);
> + guard(spinlock_irqsave)(&bdata->lock);
>
> if (!bdata->key_pressed) {
> if (bdata->button->wakeup)
> @@ -497,7 +488,6 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id)
> ms_to_ktime(bdata->release_delay),
> HRTIMER_MODE_REL_HARD);
> out:
> - spin_unlock_irqrestore(&bdata->lock, flags);
> return IRQ_HANDLED;
> }
>
> @@ -1062,10 +1052,10 @@ static int gpio_keys_suspend(struct device *dev)
> if (error)
> return error;
> } else {
> - mutex_lock(&input->mutex);
> + guard(mutex)(&input->mutex);
> +
> if (input_device_enabled(input))
> gpio_keys_close(input);
> - mutex_unlock(&input->mutex);
> }
>
> return 0;
> @@ -1075,20 +1065,20 @@ static int gpio_keys_resume(struct device *dev)
> {
> struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
> struct input_dev *input = ddata->input;
> - int error = 0;
> + int error;
>
> if (device_may_wakeup(dev)) {
> gpio_keys_disable_wakeup(ddata);
> } else {
> - mutex_lock(&input->mutex);
> - if (input_device_enabled(input))
> + guard(mutex)(&input->mutex);
> +
> + if (input_device_enabled(input)) {
> error = gpio_keys_open(input);
> - mutex_unlock(&input->mutex);
> + if (error)
> + return error;
> + }
> }
>
> - if (error)
> - return error;
> -
> gpio_keys_report_state(ddata);
> return 0;
> }