The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
@ 2026-05-06 15:26 Sajal Gupta
  2026-05-09  1:17 ` Jakub Kicinski
  2026-05-09  1:17 ` Jakub Kicinski
  0 siblings, 2 replies; 3+ messages in thread
From: Sajal Gupta @ 2026-05-06 15:26 UTC (permalink / raw)
  To: petkan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-usb, netdev,
	linux-kernel, Sajal Gupta

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
  2026-05-06 15:26 [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint Sajal Gupta
@ 2026-05-09  1:17 ` Jakub Kicinski
  2026-05-09  1:17 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-05-09  1:17 UTC (permalink / raw)
  To: sajal2005gupta
  Cc: Jakub Kicinski, petkan, andrew+netdev, davem, edumazet, pabeni,
	linux-usb, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint
  2026-05-06 15:26 [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint Sajal Gupta
  2026-05-09  1:17 ` Jakub Kicinski
@ 2026-05-09  1:17 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-05-09  1:17 UTC (permalink / raw)
  To: Sajal Gupta
  Cc: petkan, andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
	linux-kernel

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))

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-09  1:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 15:26 [PATCH net-next] net: usb: pegasus: replace simple_strtoul with kstrtouint Sajal Gupta
2026-05-09  1:17 ` Jakub Kicinski
2026-05-09  1:17 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox