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>

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>
Dmitry Torokhov · Matthew Garrett
1 parent 614ef43222
Showing 1 changed file with 65 additions and 111 deletions Side-by-side Diff
drivers/platform/x86/wmi.c
@@ -68,7 +68,7 @@
 	acpi_handle handle;
 	wmi_notify_handler handler;
 	void *handler_data;
-	struct device *dev;
+	struct device dev;
 };
  
  
@@ -110,7 +110,7 @@
 		.add = acpi_wmi_add,
 		.remove = acpi_wmi_remove,
 		.notify = acpi_wmi_notify,
-		},
+	},
 };
  
 /*
@@ -693,7 +693,9 @@
  
 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,106 +705,62 @@
 	.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);
-	}
+	list_for_each_entry_safe(wblock, next, &wmi_block_list, list)
+		device_unregister(&wblock->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;
-}
-
 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);
-	}
-}
-
 /*
  * Parse the _WDG method for the GUID data blocks
  */
  
  
  
  
@@ -814,19 +772,19 @@
 	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 @@
 			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 @@
 {
 	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_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 @@
 		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) {
+	error = class_register(&wmi_class);
+	if (error)
+		return error;
+
+	error = acpi_bus_register_driver(&acpi_wmi_driver);
+	if (error) {
 		pr_err("Error loading mapper\n");
-		return -ENODEV;
+		class_unregister(&wmi_class);
+		return error;
 	}
  
-	result = wmi_class_init();
-	if (result) {
-		acpi_bus_unregister_driver(&acpi_wmi_driver);
-		return result;
-	}
-
 	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");
 }
...	...	@@ -68,7 +68,7 @@
68	68	acpi_handle handle;
69	69	wmi_notify_handler handler;
70	70	void *handler_data;
71		- struct device *dev;
	71	+ struct device dev;
72	72	};
73	73
74	74
...	...	@@ -110,7 +110,7 @@
110	110	.add = acpi_wmi_add,
111	111	.remove = acpi_wmi_remove,
112	112	.notify = acpi_wmi_notify,
113		- },
	113	+ },
114	114	};
115	115
116	116	/*
...	...	@@ -693,7 +693,9 @@
693	693
694	694	static void wmi_dev_free(struct device *dev)
695	695	{
696		- kfree(dev);
	696	+ struct wmi_block *wmi_block = container_of(dev, struct wmi_block, dev);
	697	+
	698	+ kfree(wmi_block);
697	699	}
698	700
699	701	static struct class wmi_class = {
700	702
701	703
702	704
703	705
704	706
705	707
706	708
707	709
708	710
709	711
710	712
711	713
712	714
713	715
714	716
715	717
716	718
717	719
718	720
...	...	@@ -703,106 +705,62 @@
703	705	.dev_attrs = wmi_dev_attrs,
704	706	};
705	707
706		-static int wmi_create_devs(void)
	708	+static struct wmi_block wmi_create_device(const struct guid_block gblock,
	709	+ acpi_handle handle)
707	710	{
708		- int result;
709		- char guid_string[37];
710		- struct guid_block *gblock;
711	711	struct wmi_block *wblock;
712		- struct list_head *p;
713		- struct device *guid_dev;
	712	+ int error;
	713	+ char guid_string[37];
714	714
715		- /* Create devices for all the GUIDs */
716		- list_for_each(p, &wmi_block_list) {
717		- wblock = list_entry(p, struct wmi_block, list);
	715	+ wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
	716	+ if (!wblock) {
	717	+ error = -ENOMEM;
	718	+ goto err_out;
	719	+ }
718	720
719		- guid_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
720		- if (!guid_dev)
721		- return -ENOMEM;
	721	+ wblock->handle = handle;
	722	+ wblock->gblock = *gblock;
722	723
723		- wblock->dev = guid_dev;
	724	+ wblock->dev.class = &wmi_class;
724	725
725		- guid_dev->class = &wmi_class;
726		- dev_set_drvdata(guid_dev, wblock);
	726	+ wmi_gtoa(gblock->guid, guid_string);
	727	+ dev_set_name(&wblock->dev, guid_string);
727	728
728		- gblock = &wblock->gblock;
	729	+ dev_set_drvdata(&wblock->dev, wblock);
729	730
730		- wmi_gtoa(gblock->guid, guid_string);
731		- dev_set_name(guid_dev, guid_string);
	731	+ error = device_register(&wblock->dev);
	732	+ if (error)
	733	+ goto err_free;
732	734
733		- result = device_register(guid_dev);
734		- if (result)
735		- return result;
736		- }
	735	+ list_add_tail(&wblock->list, &wmi_block_list);
	736	+ return wblock;
737	737
738		- return 0;
	738	+err_free:
	739	+ kfree(wblock);
	740	+err_out:
	741	+ return ERR_PTR(error);
739	742	}
740	743
741		-static void wmi_remove_devs(void)
	744	+static void wmi_free_devices(void)
742	745	{
743		- struct guid_block *gblock;
744		- struct wmi_block *wblock;
745		- struct list_head *p;
746		- struct device *guid_dev;
	746	+ struct wmi_block wblock, next;
747	747
748	748	/* Delete devices for all the GUIDs */
749		- list_for_each(p, &wmi_block_list) {
750		- wblock = list_entry(p, struct wmi_block, list);
751		-
752		- guid_dev = wblock->dev;
753		- gblock = &wblock->gblock;
754		-
755		- device_unregister(guid_dev);
756		- }
	749	+ list_for_each_entry_safe(wblock, next, &wmi_block_list, list)
	750	+ device_unregister(&wblock->dev);
757	751	}
758	752
759		-static void wmi_class_exit(void)
760		-{
761		- wmi_remove_devs();
762		- class_unregister(&wmi_class);
763		-}
764		-
765		-static int wmi_class_init(void)
766		-{
767		- int ret;
768		-
769		- ret = class_register(&wmi_class);
770		- if (ret)
771		- return ret;
772		-
773		- ret = wmi_create_devs();
774		- if (ret)
775		- wmi_class_exit();
776		-
777		- return ret;
778		-}
779		-
780	753	static bool guid_already_parsed(const char *guid_string)
781	754	{
782		- struct guid_block *gblock;
783	755	struct wmi_block *wblock;
784		- struct list_head *p;
785	756
786		- list_for_each(p, &wmi_block_list) {
787		- wblock = list_entry(p, struct wmi_block, list);
788		- gblock = &wblock->gblock;
789		-
790		- if (strncmp(gblock->guid, guid_string, 16) == 0)
	757	+ list_for_each_entry(wblock, &wmi_block_list, list)
	758	+ if (strncmp(wblock->gblock.guid, guid_string, 16) == 0)
791	759	return true;
792		- }
	760	+
793	761	return false;
794	762	}
795	763
796		-static void free_wmi_blocks(void)
797		-{
798		- struct wmi_block wblock, next;
799		-
800		- list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
801		- list_del(&wblock->list);
802		- kfree(wblock);
803		- }
804		-}
805		-
806	764	/*
807	765	* Parse the _WDG method for the GUID data blocks
808	766	*/
809	767
810	768
811	769
812	770
...	...	@@ -814,19 +772,19 @@
814	772	struct wmi_block *wblock;
815	773	char guid_string[37];
816	774	acpi_status status;
	775	+ int retval;
817	776	u32 i, total;
818	777
819	778	status = acpi_evaluate_object(handle, "_WDG", NULL, &out);
820		-
821	779	if (ACPI_FAILURE(status))
822		- return status;
	780	+ return -ENXIO;
823	781
824	782	obj = (union acpi_object *) out.pointer;
825	783	if (!obj)
826		- return AE_ERROR;
	784	+ return -ENXIO;
827	785
828	786	if (obj->type != ACPI_TYPE_BUFFER) {
829		- status = AE_ERROR;
	787	+ retval = -ENXIO;
830	788	goto out_free_pointer;
831	789	}
832	790
833	791
834	792
835	793
836	794
837	795
...	...	@@ -846,31 +804,29 @@
846	804	pr_info("Skipping duplicate GUID %s\n", guid_string);
847	805	continue;
848	806	}
	807	+
849	808	if (debug_dump_wdg)
850	809	wmi_dump_wdg(&gblock[i]);
851	810
852		- wblock = kzalloc(sizeof(struct wmi_block), GFP_KERNEL);
853		- if (!wblock) {
854		- status = AE_NO_MEMORY;
855		- goto out_free_pointer;
	811	+ wblock = wmi_create_device(&gblock[i], handle);
	812	+ if (IS_ERR(wblock)) {
	813	+ retval = PTR_ERR(wblock);
	814	+ wmi_free_devices();
	815	+ break;
856	816	}
857	817
858		- wblock->gblock = gblock[i];
859		- wblock->handle = handle;
860	818	if (debug_event) {
861	819	wblock->handler = wmi_notify_debug;
862	820	wmi_method_enable(wblock, 1);
863	821	}
864		- list_add_tail(&wblock->list, &wmi_block_list);
865	822	}
866	823
	824	+ retval = 0;
	825	+
867	826	out_free_pointer:
868	827	kfree(out.pointer);
869	828
870		- if (ACPI_FAILURE(status))
871		- free_wmi_blocks();
872		-
873		- return status;
	829	+ return retval;
874	830	}
875	831
876	832	/*
...	...	@@ -949,6 +905,7 @@
949	905	{
950	906	acpi_remove_address_space_handler(device->handle,
951	907	ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
	908	+ wmi_free_devices();
952	909
953	910	return 0;
954	911	}
...	...	@@ -956,7 +913,7 @@
956	913	static int acpi_wmi_add(struct acpi_device *device)
957	914	{
958	915	acpi_status status;
959		- int result = 0;
	916	+ int error;
960	917
961	918	status = acpi_install_address_space_handler(device->handle,
962	919	ACPI_ADR_SPACE_EC,
963	920
964	921
965	922
966	923
967	924
968	925
969	926
970	927
971	928
...	...	@@ -967,47 +924,44 @@
967	924	return -ENODEV;
968	925	}
969	926
970		- status = parse_wdg(device->handle);
971		- if (ACPI_FAILURE(status)) {
	927	+ error = parse_wdg(device->handle);
	928	+ if (error) {
972	929	acpi_remove_address_space_handler(device->handle,
973	930	ACPI_ADR_SPACE_EC,
974	931	&acpi_wmi_ec_space_handler);
975	932	pr_err("Failed to parse WDG method\n");
976		- return -ENODEV;
	933	+ return error;
977	934	}
978	935
979		- return result;
	936	+ return 0;
980	937	}
981	938
982	939	static int __init acpi_wmi_init(void)
983	940	{
984		- int result;
	941	+ int error;
985	942
986	943	if (acpi_disabled)
987	944	return -ENODEV;
988	945
989		- result = acpi_bus_register_driver(&acpi_wmi_driver);
990		- if (result < 0) {
	946	+ error = class_register(&wmi_class);
	947	+ if (error)
	948	+ return error;
	949	+
	950	+ error = acpi_bus_register_driver(&acpi_wmi_driver);
	951	+ if (error) {
991	952	pr_err("Error loading mapper\n");
992		- return -ENODEV;
	953	+ class_unregister(&wmi_class);
	954	+ return error;
993	955	}
994	956
995		- result = wmi_class_init();
996		- if (result) {
997		- acpi_bus_unregister_driver(&acpi_wmi_driver);
998		- return result;
999		- }
1000		-
1001	957	pr_info("Mapper loaded\n");
1002		-
1003	958	return 0;
1004	959	}
1005	960
1006	961	static void __exit acpi_wmi_exit(void)
1007	962	{
1008		- wmi_class_exit();
1009	963	acpi_bus_unregister_driver(&acpi_wmi_driver);
1010		- free_wmi_blocks();
	964	+ class_unregister(&wmi_class);
1011	965
1012	966	pr_info("Mapper unloaded\n");
1013	967	}