Re: [PATCH v9 4/4] HID: hid-msi: Add Rumble Intensity Attributes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Mixing goto-based error handling with scope-based cleanup (__free/guard) in claw_hw_output_report().
--

commit a152767539ab279fa2750588503cc5a77a8522da
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add Rumble Intensity Attributes

Adds intensity adjustment for the left and right rumble motors. It introduces
new sysfs attributes for reading and writing rumble intensity.

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 96710d80543d4..f5c54cd62f3bb 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
[ ... ]
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
[ ... ]
> +	guard(mutex)(&drvdata->rom_mutex);
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> +				    (u8 *)&report, sizeof(report), 25);

[Severity: High]
This is a pre-existing issue, but does claw_hw_output_report() mix goto-based
error handling with scope-based cleanup?

Looking at claw_hw_output_report(), it defines dmabuf with __free(kfree)
and uses guard(mutex), but also relies on explicit goto err_clear_state
jumps for error handling:

    unsigned char *dmabuf __free(kfree) = NULL;
    ...
    if (ret < 0)
        goto err_clear_state;
    ...
    err_clear_state:

According to the cleanup guidelines, functions should either convert all
resources to scope-based cleanup, or convert none of them to avoid confusing
ownership semantics. Could this function be updated to strictly use one
approach?

> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525020543.519082-1-derekjohn.clark@gmail.com?part=4