[PATCH v2 3/4] HID: wiimote: use scoped cleanup in wiimote and led probes

[Rafael Passos <rafael@rcpassos.me>]

Cleanup code in wiimote/led probe function, using the scoped cleanup.
This prevents mistakes in future changes to this function.

In wiimote_probe_clenaup, a few functions are safe to call without
checking. For the hid_hw calls, a new bit mask was introduced to track
probing state.

Signed-off-by: Rafael Passos <rafael@rcpassos.me>
---
 drivers/hid/hid-wiimote-core.c    | 68 ++++++++++++++++++-------------
 drivers/hid/hid-wiimote-modules.c | 17 ++++----
 drivers/hid/hid-wiimote.h         |  1 +
 3 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index d37d740a7162..b549a7d58d51 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -1774,16 +1774,40 @@ static void wiimote_destroy(struct wiimote_data *wdata)
 /* Global id allocator for wii remotes */
 static DEFINE_IDA(wiimote_ida);
 
+#define WIIMOTE_PROBE_HW_STARTED  BIT(0)  // hid_hw_start succeeded
+#define WIIMOTE_PROBE_HW_OPENED   BIT(1)  // hid_hw_open succeeded
+
+static void __wiimote_probe_cleanup(struct wiimote_data *wdata)
+{
+	if (!wdata)
+		return;
+
+	if (wdata->player_id)
+		ida_free(&wiimote_ida, wdata->player_id);
+
+	// safe, debugfs checks IS_ERR_OR_NULL
+	wiidebug_deinit(wdata);
+	// safe, checks dev for NULL
+	device_remove_file(&wdata->hdev->dev, &dev_attr_devtype);
+	device_remove_file(&wdata->hdev->dev, &dev_attr_extension);
+	if (wdata->probe_state & WIIMOTE_PROBE_HW_OPENED)
+		hid_hw_close(wdata->hdev);
+	if (wdata->probe_state & WIIMOTE_PROBE_HW_STARTED)
+		hid_hw_stop(wdata->hdev);
+	kfree(wdata);
+}
+
+DEFINE_FREE(wiimote_probe_cleanup, struct wiimote_data *,
+	__wiimote_probe_cleanup(_T))
+
 static int wiimote_hid_probe(struct hid_device *hdev,
 				const struct hid_device_id *id)
 {
-	struct wiimote_data *wdata;
 	int ret;
-	int player_id;
 
 	hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
 
-	wdata = wiimote_create(hdev);
+	struct wiimote_data *wdata __free(wiimote_probe_cleanup) = wiimote_create(hdev);
 	if (!wdata) {
 		hid_err(hdev, "Can't alloc device\n");
 		return -ENOMEM;
@@ -1792,68 +1816,54 @@ static int wiimote_hid_probe(struct hid_device *hdev,
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "HID parse failed\n");
-		goto err;
+		return ret;
 	}
 
 	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
 	if (ret) {
 		hid_err(hdev, "HW start failed\n");
-		goto err;
+		return ret;
 	}
+	wdata->probe_state |= WIIMOTE_PROBE_HW_STARTED;
 
 	ret = hid_hw_open(hdev);
 	if (ret) {
 		hid_err(hdev, "cannot start hardware I/O\n");
-		goto err_stop;
+		return ret;
 	}
+	wdata->probe_state |= WIIMOTE_PROBE_HW_OPENED;
 
 	ret = device_create_file(&hdev->dev, &dev_attr_extension);
 	if (ret) {
 		hid_err(hdev, "cannot create sysfs attribute\n");
-		goto err_close;
+		return ret;
 	}
 
 	ret = device_create_file(&hdev->dev, &dev_attr_devtype);
 	if (ret) {
 		hid_err(hdev, "cannot create sysfs attribute\n");
-		goto err_ext;
+		return ret;
 	}
 
 	ret = wiidebug_init(wdata);
 	if (ret)
-		goto err_free;
+		return ret;
 
-	player_id = ida_alloc_min(&wiimote_ida, 1, GFP_KERNEL);
+	int player_id = ida_alloc_min(&wiimote_ida, 1, GFP_KERNEL);
 	if (player_id < 1) {
 		hid_err(hdev, "cannot allocate controller id\n");
 		ret = player_id;
-		goto err_free;
+		return ret;
 	}
-
 	wdata->player_id = player_id;
 
+
 	hid_info(hdev, "New device registered (Wiimote %d)\n", player_id);
 
 	/* schedule device detection */
 	wiimote_schedule(wdata);
-
+	retain_and_null_ptr(wdata);
 	return 0;
-
-err_free:
-	wiimote_destroy(wdata);
-	return ret;
-
-err_ext:
-	device_remove_file(&wdata->hdev->dev, &dev_attr_extension);
-err_close:
-	hid_hw_close(hdev);
-err_stop:
-	hid_hw_stop(hdev);
-err:
-	input_free_device(wdata->ir);
-	input_free_device(wdata->accel);
-	kfree(wdata);
-	return ret;
 }
 
 static void wiimote_hid_remove(struct hid_device *hdev)
diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c
index 3cd614466740..47fa6a8ecdae 100644
--- a/drivers/hid/hid-wiimote-modules.c
+++ b/drivers/hid/hid-wiimote-modules.c
@@ -341,11 +341,11 @@ static int wiimod_led_probe(const struct wiimod_ops *ops,
 {
 	struct device *dev = &wdata->hdev->dev;
 	size_t namesz = strlen(dev_name(dev)) + 9;
-	struct led_classdev *led;
 	char *name;
 	int ret;
 
-	led = kzalloc(sizeof(struct led_classdev) + namesz, GFP_KERNEL);
+	struct led_classdev *led __free(kfree) =
+		kzalloc(sizeof(struct led_classdev) + namesz, GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
 
@@ -359,8 +359,12 @@ static int wiimod_led_probe(const struct wiimod_ops *ops,
 
 	wdata->leds[ops->arg] = led;
 	ret = led_classdev_register(dev, led);
-	if (ret)
-		goto err_free;
+	if (ret) {
+		wdata->leds[ops->arg] = NULL;
+		return ret;
+	}
+
+	retain_and_null_ptr(led);
 
 	/* enable LED1 to stop initial LED-blinking */
 	if (ops->arg == 0) {
@@ -369,11 +373,6 @@ static int wiimod_led_probe(const struct wiimod_ops *ops,
 	}
 
 	return 0;
-
-err_free:
-	wdata->leds[ops->arg] = NULL;
-	kfree(led);
-	return ret;
 }
 
 static void wiimod_led_remove(const struct wiimod_ops *ops,
diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
index a53f72d5077e..6812efa589c9 100644
--- a/drivers/hid/hid-wiimote.h
+++ b/drivers/hid/hid-wiimote.h
@@ -154,6 +154,7 @@ struct wiimote_data {
 	struct timer_list timer;
 	struct wiimote_debug *debug;
 	__u8 player_id;
+	__u8 probe_state;
 
 	union {
 		struct input_dev *input;
-- 
2.53.0