WMI: embed struct device directly into wmi_block

Instead of creating wmi_blocks and then register corresponding devices
on a separate pass do it all in one shot, since lifetime rules for both
objects are the same. This also takes care of leaking devices when
device_create fails for one of them.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Matthew Garrett <mjg@redhat.com>

1 files changed, 65 insertions, 111 deletions

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index b9a60a0..104b77c 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -68,7 +68,7 @@ struct wmi_block {
 	acpi_handle handle;
 	wmi_notify_handler handler;
 	void *handler_data;
-	struct device *dev;
+	struct device dev;
 };
 
 
@@ -110,7 +110,7 @@ static struct acpi_driver acpi_wmi_driver = {
 		.add = acpi_wmi_add,
 		.remove = acpi_wmi_remove,
 		.notify = acpi_wmi_notify,
-		},
+	},
 };
 
 /*
@@ -693,7 +693,9 @@ static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 static void wmi_dev_free(struct device *dev)
 {
-	kfree(dev);
+	struct wmi_block *wmi_block = container_of(dev, struct wmi_block, dev);
+
+	kfree(wmi_block);
 }
 
 static struct class wmi_class = {
@@ -703,104 +705,60 @@ static struct class wmi_class = {
 	.dev_attrs = wmi_dev_attrs,
 };
 
-static int wmi_create_devs(void)
+static struct wmi_block *wmi_create_device(const struct guid_block *gblock,
+					   acpi_handle handle)
 {
-	int result;
-	char guid_string[37];
-	struct guid_block *gblock;
 	struct wmi_block *wblock;
-	struct list_head *p;
-	struct device *guid_dev;
+	int error;
+	char guid_string[37];
 
-	/* Create devices for all the GUIDs */
-	list_for_each(p, &wmi_block_list) {
-		wblock = list_entry(p, struct wmi_block, list);
+	wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
+	if (!wblock) {
+		error = -ENOMEM;
+		goto err_out;
+	}
 
-		guid_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
-		if (!guid_dev)
-			return -ENOMEM;
+	wblock->handle = handle;
+	wblock->gblock = *gblock;
 
-		wblock->dev = guid_dev;
+	wblock->dev.class = &wmi_class;
 
-		guid_dev->class = &wmi_class;
-		dev_set_drvdata(guid_dev, wblock);
+	wmi_gtoa(gblock->guid, guid_string);
+	dev_set_name(&wblock->dev, guid_string);
 
-		gblock = &wblock->gblock;
+	dev_set_drvdata(&wblock->dev, wblock);
 
-		wmi_gtoa(gblock->guid, guid_string);
-		dev_set_name(guid_dev, guid_string);
+	error = device_register(&wblock->dev);
+	if (error)
+		goto err_free;
 
-		result = device_register(guid_dev);
-		if (result)
-			return result;
-	}
+	list_add_tail(&wblock->list, &wmi_block_list);
+	return wblock;
 
-	return 0;
+err_free:
+	kfree(wblock);
+err_out:
+	return ERR_PTR(error);
 }
 
-static void wmi_remove_devs(void)
+static void wmi_free_devices(void)
 {
-	struct guid_block *gblock;
-	struct wmi_block *wblock;
-	struct list_head *p;
-	struct device *guid_dev;
+	struct wmi_block *wblock, *next;
 
 	/* Delete devices for all the GUIDs */
-	list_for_each(p, &wmi_block_list) {
-		wblock = list_entry(p, struct wmi_block, list);
-
-		guid_dev = wblock->dev;
-		gblock = &wblock->gblock;
-
-		device_unregister(guid_dev);
-	}
-}
-
-static void wmi_class_exit(void)
-{
-	wmi_remove_devs();
-	class_unregister(&wmi_class);
-}
-
-static int wmi_class_init(void)
-{
-	int ret;
-
-	ret = class_register(&wmi_class);
-	if (ret)
-		return ret;
-
-	ret = wmi_create_devs();
-	if (ret)
-		wmi_class_exit();
-
-	return ret;
+	list_for_each_entry_safe(wblock, next, &wmi_block_list, list)
+		device_unregister(&wblock->dev);
 }
 
 static bool guid_already_parsed(const char *guid_string)
 {
-	struct guid_block *gblock;
 	struct wmi_block *wblock;
-	struct list_head *p;
-
-	list_for_each(p, &wmi_block_list) {
-		wblock = list_entry(p, struct wmi_block, list);
-		gblock = &wblock->gblock;
 
-		if (strncmp(gblock->guid, guid_string, 16) == 0)
+	list_for_each_entry(wblock, &wmi_block_list, list)
+		if (strncmp(wblock->gblock.guid, guid_string, 16) == 0)
 			return true;
-	}
-	return false;
-}
 
-static void free_wmi_blocks(void)
-{
-	struct wmi_block *wblock, *next;
-
-	list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
-		list_del(&wblock->list);
-		kfree(wblock);
-	}
+	return false;
 }
 
 /*
@@ -814,19 +772,19 @@ static acpi_status parse_wdg(acpi_handle handle)
 	struct wmi_block *wblock;
 	char guid_string[37];
 	acpi_status status;
+	int retval;
 	u32 i, total;
 
 	status = acpi_evaluate_object(handle, "_WDG", NULL, &out);
-
 	if (ACPI_FAILURE(status))
-		return status;
+		return -ENXIO;
 
 	obj = (union acpi_object *) out.pointer;
 	if (!obj)
-		return AE_ERROR;
+		return -ENXIO;
 
 	if (obj->type != ACPI_TYPE_BUFFER) {
-		status = AE_ERROR;
+		retval = -ENXIO;
 		goto out_free_pointer;
 	}
 
@@ -846,31 +804,29 @@ static acpi_status parse_wdg(acpi_handle handle)
 			pr_info("Skipping duplicate GUID %s\n", guid_string);
 			continue;
 		}
+
 		if (debug_dump_wdg)
 			wmi_dump_wdg(&gblock[i]);
 
-		wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
-		if (!wblock) {
-			status = AE_NO_MEMORY;
-			goto out_free_pointer;
+		wblock = wmi_create_device(&gblock[i], handle);
+		if (IS_ERR(wblock)) {
+			retval = PTR_ERR(wblock);
+			wmi_free_devices();
+			break;
 		}
 
-		wblock->gblock = gblock[i];
-		wblock->handle = handle;
 		if (debug_event) {
 			wblock->handler = wmi_notify_debug;
 			wmi_method_enable(wblock, 1);
 		}
-		list_add_tail(&wblock->list, &wmi_block_list);
 	}
 
+	retval = 0;
+
 out_free_pointer:
 	kfree(out.pointer);
 
-	if (ACPI_FAILURE(status))
-		free_wmi_blocks();
-
-	return status;
+	return retval;
 }
 
 /*
@@ -949,6 +905,7 @@ static int acpi_wmi_remove(struct acpi_device *device, int type)
 {
 	acpi_remove_address_space_handler(device->handle,
 				ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
+	wmi_free_devices();
 
 	return 0;
 }
@@ -956,7 +913,7 @@ static int acpi_wmi_remove(struct acpi_device *device, int type)
 static int acpi_wmi_add(struct acpi_device *device)
 {
 	acpi_status status;
-	int result = 0;
+	int error;
 
 	status = acpi_install_address_space_handler(device->handle,
 						    ACPI_ADR_SPACE_EC,
@@ -967,47 +924,44 @@ static int acpi_wmi_add(struct acpi_device *device)
 		return -ENODEV;
 	}
 
-	status = parse_wdg(device->handle);
-	if (ACPI_FAILURE(status)) {
+	error = parse_wdg(device->handle);
+	if (error) {
 		acpi_remove_address_space_handler(device->handle,
 						  ACPI_ADR_SPACE_EC,
 						  &acpi_wmi_ec_space_handler);
 		pr_err("Failed to parse WDG method\n");
-		return -ENODEV;
+		return error;
 	}
 
-	return result;
+	return 0;
 }
 
 static int __init acpi_wmi_init(void)
 {
-	int result;
+	int error;
 
 	if (acpi_disabled)
 		return -ENODEV;
 
-	result = acpi_bus_register_driver(&acpi_wmi_driver);
-	if (result < 0) {
-		pr_err("Error loading mapper\n");
-		return -ENODEV;
-	}
+	error = class_register(&wmi_class);
+	if (error)
+		return error;
 
-	result = wmi_class_init();
-	if (result) {
-		acpi_bus_unregister_driver(&acpi_wmi_driver);
-		return result;
+	error = acpi_bus_register_driver(&acpi_wmi_driver);
+	if (error) {
+		pr_err("Error loading mapper\n");
+		class_unregister(&wmi_class);
+		return error;
 	}
 
 	pr_info("Mapper loaded\n");
-
 	return 0;
 }
 
 static void __exit acpi_wmi_exit(void)
 {
-	wmi_class_exit();
 	acpi_bus_unregister_driver(&acpi_wmi_driver);
-	free_wmi_blocks();
+	class_unregister(&wmi_class);
 
 	pr_info("Mapper unloaded\n");
 }