Re: [PATCH v13 05/18] HID: asus: add joysticks inner and outer range configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] HID: asus: ally_prefs_setup() mixes scoped_guard() with goto error handling
--

commit 23b7fd753beca8abc912e5e22d7a6deed94442a0
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: add joysticks inner and outer range configuration

This commit adds sysfs attributes for configuring the inner and outer
joystick thresholds on ASUS ROG Ally devices and restores those threshold
settings on device initialization.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 459061391c378..a89ce83ae93f4 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -1182,15 +1401,44 @@ static int ally_prefs_setup(struct hid_device *hdev, struct ally_intf *intf)
>  		}
>  	}
>  
> +	scoped_guard(mutex, &ally_prefs.lock)
> +		cal_live = ally_prefs.user_cal_support ||
> +			ally_prefs.anti_deadzone_support ||
> +			ally_prefs.resp_curve_support;
> +
> +	/* Skip the calibration groups when the capability is missing. */
> +	if (cal_live) {
> +		for (; cal_n < ARRAY_SIZE(ally_cal_attr_groups); cal_n++) {
> +			ret = sysfs_create_group(&hdev->dev.kobj,
> +						ally_cal_attr_groups[cal_n]);
> +			if (ret) {
> +				hid_err(hdev, "Failed to create Ally sysfs group '%s': %d\n",
> +					ally_cal_attr_groups[cal_n]->name, ret);
> +				goto err_cal_groups;
> +			}
> +		}
> +	}
> +
>  	scoped_guard(mutex, &ally_prefs.lock) {
> +		intf->cal_sysfs_live = cal_live;
>  		intf->sysfs_live = true;
>  	}
>  
>  	return 0;
> +
> +err_cal_groups:
> +	while (cal_n--)
> +		sysfs_remove_group(&hdev->dev.kobj, ally_cal_attr_groups[cal_n]);
> +
> +	for (i = 0; i < ARRAY_SIZE(ally_attr_groups); i++)
> +		sysfs_remove_group(&hdev->dev.kobj, &ally_attr_groups[i]);
> +
> +	return ret;
>  }

[Severity: Low]
This isn't a bug, but ally_prefs_setup() uses scoped_guard() while also
introducing a goto err_cal_groups error path when sysfs_create_group()
fails for ally_cal_attr_groups.

Since the cleanup API guidelines discourage mixing scope-based cleanup
helpers with goto-based error handling in the same function, could the
unwinding be done inline (similar to the earlier ally_attr_groups loop in
ally_prefs_setup()), or by using sysfs_create_groups() and
sysfs_remove_groups() with a NULL-terminated array to avoid the goto label?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104013935.512713-1-denis.benato@linux.dev?part=5