diff mbox series

[05/17] Input: gpio-keys - switch to using cleanup functions

Message ID 20240825051627.2848495-6-dmitry.torokhov@gmail.com
State Accepted
Commit 43a365abaf47539f065f6c7890e5433ef10f7381
Headers show
Series Convert keyboard drivers to use new cleanup facilities | expand

Commit Message

Dmitry Torokhov Aug. 25, 2024, 5:16 a.m. UTC
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>
---
 drivers/input/keyboard/gpio_keys.c | 44 ++++++++++++------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

Comments

Hans de Goede Aug. 25, 2024, 1:10 p.m. UTC | #1
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;
>  }
diff mbox series

Patch

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;
 }