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

[Jakub Kicinski]

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