[PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint

Sajal Gupta posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/net/usb/pegasus.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
[PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
Posted by Sajal Gupta 1 month, 1 week ago
simple_strtoul() is deprecated as it has no error checking. Replace it
with kstrtouint() which returns an error code on invalid input, and add
appropriate error handling.

Also add a NULL check before parsing flags, since strsep() can set id
to NULL if the input has fewer tokens than expected.

Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
---
 drivers/net/usb/pegasus.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index db85f40734d7..d45f08f8f22e 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -1327,15 +1327,29 @@ static void __init parse_id(char *id)
 {
 	unsigned int vendor_id = 0, device_id = 0, flags = 0, i = 0;
 	char *token, *name = NULL;
+	int err;
 
-	if ((token = strsep(&id, ":")) != NULL)
+	token = strsep(&id, ":");
+	if (token)
 		name = token;
 	/* name now points to a null terminated string*/
-	if ((token = strsep(&id, ":")) != NULL)
-		vendor_id = simple_strtoul(token, NULL, 16);
-	if ((token = strsep(&id, ":")) != NULL)
-		device_id = simple_strtoul(token, NULL, 16);
-	flags = simple_strtoul(id, NULL, 16);
+	token = strsep(&id, ":");
+	if (token) {
+		err = kstrtouint(token, 16, &vendor_id);
+		if (err)
+			return;
+	}
+	token = strsep(&id, ":");
+	if (token) {
+		err = kstrtouint(token, 16, &device_id);
+		if (err)
+			return;
+	}
+	if (id) {
+		err = kstrtouint(id, 16, &flags);
+		if (err)
+			return;
+	}
 	pr_info("%s: new device %s, vendor ID 0x%04x, device ID 0x%04x, flags: 0x%x\n",
 		driver_name, name, vendor_id, device_id, flags);
 
-- 
2.54.0
Re: [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
Posted by Jakub Kicinski 1 month ago
On Wed,  6 May 2026 20:56:36 +0530 Sajal Gupta wrote:
> +	token = strsep(&id, ":");
> +	if (token) {
> +		err = kstrtouint(token, 16, &vendor_id);
> +		if (err)
> +			return;
> +	}

Why bother capturing err if we can't return it?

if (token && kstrtouint(token, 16, &vendor_id))
Re: [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
Posted by Jakub Kicinski 1 month ago
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: usb: pegasus: replace simple_strtoul with kstrtouint

This patch replaces the deprecated simple_strtoul() function with
kstrtouint() when parsing device IDs in the pegasus driver. It also
adds error checking to abort on invalid inputs.

> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index db85f40734d7b..d45f08f8f22ee 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
[ ... ]
> @@ -1327,15 +1327,29 @@ static void __init parse_id(char *id)
>  {
[ ... ]
> -	if ((token = strsep(&id, ":")) != NULL)
> -		device_id = simple_strtoul(token, NULL, 16);
> -	flags = simple_strtoul(id, NULL, 16);
> +	token = strsep(&id, ":");
> +	if (token) {
> +		err = kstrtouint(token, 16, &device_id);
> +		if (err)
> +			return;
> +	}
> +	if (id) {
> +		err = kstrtouint(id, 16, &flags);
> +		if (err)
> +			return;

Does this stricter parsing silently break previously working module parameter
formats?

In the original code, simple_strtoul() would leniently parse an empty string
as 0. If a user provided a trailing colon to omit the flags parameter
(for example, name:1234:5678:), id becomes an empty string.

Since kstrtouint() strictly rejects empty strings and returns -EINVAL,
this will now take the error path and return. Because there is no logging
on this error path, device registration will be silently aborted.

Could this be updated to either handle empty strings or log a warning so
users know why their configuration stopped working?

> +	}
>  	pr_info("%s: new device %s, vendor ID 0x%04x, device ID 0x%04x, flags: 0x%x\n",
>  		driver_name, name, vendor_id, device_id, flags);
-- 
pw-bot: cr