linux-kernel - Re: [PATCH] drivers: input: Directly use ida

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 May 2022 12:28:24 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     keliu <liuke94@...wei.com>, dmitry.torokhov@...il.com,
        marcoshalano@...il.com, michael@...haelcullen.name,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: input: Directly use ida_alloc()/free()

Hi,

Le 27/05/2022 à 12:37, keliu a écrit :
> Use ida_alloc()/ida_free() instead of deprecated
> ida_simple_get()/ida_simple_remove() .
> 
> Signed-off-by: keliu <liuke94@...wei.com>
> ---
>   drivers/input/input.c         | 8 ++++----
>   drivers/input/joystick/xpad.c | 6 +++---
>   2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 1365c9dfb5f2..1e4a275795f9 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -2618,15 +2618,15 @@ int input_get_new_minor(int legacy_base, unsigned int legacy_num,
>   	 * locking is needed here.
>   	 */
>   	if (legacy_base >= 0) {
> -		int minor = ida_simple_get(&input_ida,
> +		int minor = ida_alloc_range(&input_ida,
>   					   legacy_base,
> -					   legacy_base + legacy_num,
> +					   legacy_base + legacy_num - 1,

You got my point, things are going in the right direction.
This one is correct...

>   					   GFP_KERNEL);
>   		if (minor >= 0 || !allow_dynamic)
>   			return minor;
>   	}
>   
> -	return ida_simple_get(&input_ida,
> +	return ida_alloc_range(&input_ida,
>   			      INPUT_FIRST_DYNAMIC_DEV, INPUT_MAX_CHAR_DEVICES,

... but you missed the -1 here.

Also maybe an explanation of why this -1 are introduced would help 
reviewers. (if needed, I think I already wrote some, just ask)

CJ

>   			      GFP_KERNEL);
>   }
> @@ -2641,7 +2641,7 @@ EXPORT_SYMBOL(input_get_new_minor);
>    */
>   void input_free_minor(unsigned int minor)
>   {
> -	ida_simple_remove(&input_ida, minor);
> +	ida_free(&input_ida, minor);
>   }
>   EXPORT_SYMBOL(input_free_minor);
>   
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 18190b529bca..fafc0d5703dc 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1456,7 +1456,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>   	if (!led)
>   		return -ENOMEM;
>   
> -	xpad->pad_nr = ida_simple_get(&xpad_pad_seq, 0, 0, GFP_KERNEL);
> +	xpad->pad_nr = ida_alloc(&xpad_pad_seq, GFP_KERNEL);
>   	if (xpad->pad_nr < 0) {
>   		error = xpad->pad_nr;
>   		goto err_free_mem;
> @@ -1479,7 +1479,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
>   	return 0;
>   
>   err_free_id:
> -	ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
> +	ida_free(&xpad_pad_seq, xpad->pad_nr);
>   err_free_mem:
>   	kfree(led);
>   	xpad->led = NULL;
> @@ -1492,7 +1492,7 @@ static void xpad_led_disconnect(struct usb_xpad *xpad)
>   
>   	if (xpad_led) {
>   		led_classdev_unregister(&xpad_led->led_cdev);
> -		ida_simple_remove(&xpad_pad_seq, xpad->pad_nr);
> +		ida_free(&xpad_pad_seq, xpad->pad_nr);
>   		kfree(xpad_led);
>   	}
>   }