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>

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index b9a60a0aabfded4d15e95d4f5c2c5759ad109809..104b77c87ef5ae36e0c4502c6487e1fb3aac428e 100644 (file)
--- 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");
 }