Re: [PATCH v2 3/6] HID: steam: Fix wording of connect/disconnect logs

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Device sending an empty serial number causes silent resource leaks upon disconnection.
- [Medium] Registration failure leaves `steam->serial_no` populated, permanently breaking subsequent connections.
--

--- Patch [3]: [PATCH v2 3/6] HID: steam: Fix wording of connect/disconnect logs ---
commit 1846261fd57667e998e1416cd9dcdbedc57da4e8
Author: Vicki Pfau <[email protected]>

HID: steam: Fix wording of connect/disconnect logs

This commit updates the connect and disconnect log messages to properly
identify the device as a Deck or a Controller depending on the device quirks.
It aligns these messages with other special-cased strings in the driver.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 6476fbdc58886..38c00dab0b067 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -1394,7 +1394,8 @@ static int steam_register(struct steam_device *steam)
>  			"%s:steam_get_attributes failed with error %d\n",
>  			__func__, ret);
>  
> -	hid_info(steam->hdev, "Steam Controller '%s' connected",
> +	hid_info(steam->hdev, "Steam %s '%s' connected",
> +			steam->quirks & STEAM_QUIRK_DECK ? "Deck" : "Controller",
>  			steam->serial_no);

[Severity: Medium]
This is a pre-existing issue, but can a registration failure permanently
break subsequent connections for this device?

If steam_register() fails later in the function (for example, if
steam_input_register() fails), the error path:

steam_register_sensors_fail:
	steam_input_unregister(steam);
steam_register_input_fail:
	steam_battery_unregister(steam);
	mutex_unlock(&steam->report_mutex);
	return ret;

cleans up resources but does not clear steam->serial_no.

On the next connection attempt, steam_register() will see steam->serial_no[0]
is populated and immediately return 0 without setting up any input devices,
leaving the device permanently broken until replugged.

>  
>  	/* ignore battery errors, we can live without it */
> @@ -1427,7 +1428,8 @@ static void steam_unregister(struct steam_device *steam)
>  	if (!steam->serial_no[0])
>  		return;

[Severity: High]
This is a pre-existing issue, but does this early return lead to a silent
resource leak if the device sends an empty serial number?

If steam_get_serial() successfully reads a valid HID reply but the string
payload is empty, it populates steam->serial_no with an empty string using
strscpy() and returns success, bypassing the fallback serial generation.

Later, on disconnection, steam_unregister() will hit this early return and
skip unregistering the input devices, sensors, and power supplies.

>  
> -	hid_info(steam->hdev, "Steam Controller '%s' disconnected",
> +	hid_info(steam->hdev, "Steam %s '%s' disconnected",
> +			steam->quirks & STEAM_QUIRK_DECK ? "Deck" : "Controller",
>  			steam->serial_no);
>  	steam_battery_unregister(steam);
>  	steam_sensors_unregister(steam);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3