From c64eefd48c44fa8145ad1f96edabf4a053fffc49 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Thu, 26 Aug 2010 00:15:30 -0700 Subject: [PATCH] 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 Signed-off-by: Matthew Garrett --- drivers/platform/x86/wmi.c | 176 +++++++++++++++++---------------------------- 1 file 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"); } -- 2.7.4