mbox series

[0/6] Convert joystick drivers to use new cleanup facilities

Message ID 20240904043104.1030257-1-dmitry.torokhov@gmail.com
Headers show
Series Convert joystick drivers to use new cleanup facilities | expand

Message

Dmitry Torokhov Sept. 4, 2024, 4:30 a.m. UTC 
Hi,

This series converts drivers found in drivers/input/keyboard to use new
__free() and guard() cleanup facilities that simplify the code and
ensure that all resources are released appropriately.

Thanks!

Dmitry Torokhov (6):
  Input: db9 - use guard notation when acquiring mutex
  Input: gamecon - use guard notation when acquiring mutex
  Input: iforce - use guard notation when acquiring mutex and spinlock
  Input: n64joy - use guard notation when acquiring mutex
  Input: turbografx - use guard notation when acquiring mutex
  Input: xpad - use guard notation when acquiring mutex and spinlock

 drivers/input/joystick/db9.c                  | 30 +++---
 drivers/input/joystick/gamecon.c              | 22 ++---
 drivers/input/joystick/iforce/iforce-ff.c     | 48 +++++----
 .../input/joystick/iforce/iforce-packets.c    | 57 +++++------
 drivers/input/joystick/iforce/iforce-serio.c  | 36 +++----
 drivers/input/joystick/iforce/iforce-usb.c    | 13 ++-
 drivers/input/joystick/n64joy.c               | 35 +++----
 drivers/input/joystick/turbografx.c           | 22 ++---
 drivers/input/joystick/xpad.c                 | 99 +++++++------------
 9 files changed, 152 insertions(+), 210 deletions(-)

Comments

David Lechner Nov. 20, 2024, 6:33 p.m. UTC | #1 
On 9/3/24 11:30 PM, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
> index 682a29c27832..7ac0cfc3e786 100644
> --- a/drivers/input/joystick/db9.c
> +++ b/drivers/input/joystick/db9.c
> @@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
>  {
>  	struct db9 *db9 = input_get_drvdata(dev);
>  	struct parport *port = db9->pd->port;
> -	int err;
>  
> -	err = mutex_lock_interruptible(&db9->mutex);
> -	if (err)
> -		return err;
> -
> -	if (!db9->used++) {
> -		parport_claim(db9->pd);
> -		parport_write_data(port, 0xff);
> -		if (db9_modes[db9->mode].reverse) {
> -			parport_data_reverse(port);
> -			parport_write_control(port, DB9_NORMAL);
> +	scoped_guard(mutex_intr, &db9->mutex) {
> +		if (!db9->used++) {
> +			parport_claim(db9->pd);
> +			parport_write_data(port, 0xff);
> +			if (db9_modes[db9->mode].reverse) {
> +				parport_data_reverse(port);
> +				parport_write_control(port, DB9_NORMAL);
> +			}
> +			mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
>  		}
> -		mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> +
> +		return 0;
>  	}
>  
> -	mutex_unlock(&db9->mutex);
> -	return 0;
> +	return -EINTR;

This patch and any others like it are potentially introducing a bug.