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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
[ ... ]
quoted hunk ↗ jump to hunk
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
[ ... ]
quoted hunk ↗ jump to hunk
+ 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