Skip to contentSkip to conversation

acerhdf: fix resource reclaim in error path

PATCH Superseded Axel Lin 2 participants 2 messages Jul 7, 2010

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 acerhdf: fix resource reclaim in error path

This patch fix resource reclaim in below cases:
1. acerhdf_register_platform() does not properly handle platform_device_alloc() failure and platform_device_add() failure. This patch adds error handling for acerhdf_register_platform().
2. acerhdf_register_platform() return err with acerhdf_dev == NULL. as a result, acerhdf_unregister_platform() does not do resource reclaim in acerhdf_init() error path.
This patch adds error handing for acerhdf_register_platform(), thus correct the error handing path in acerhdf_init(). goto out_err instead of err_unreg if acerhdf_register_platform() fail.
3. platform_device_del() should be only used in error handling. Current implementation missed a platform_device_put() in acerhdf_exit. This patch fixes it by using platform_device_unregister() instead of platform_device_del() in acerhdf_unregister_platform().

---
 drivers/platform/x86/acerhdf.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)
drivers/platform/x86/acerhdf.c +16 -4
index 7b2384d..e938a86 100644
@@ -579,9 +579,21 @@ static int acerhdf_register_platform(void)
 		return err;
 
 	acerhdf_dev = platform_device_alloc("acerhdf", -1);
-	platform_device_add(acerhdf_dev);
+	if (!acerhdf_dev) {
+		err = -ENOMEM;
+		goto err_device_alloc;
+	}
+	err = platform_device_add(acerhdf_dev);
+	if (err)
+		goto err_device_add;
 
 	return 0;
+
+err_device_add:
+	platform_device_put(acerhdf_dev);
+err_device_alloc:
+	platform_driver_unregister(&acerhdf_driver);
+	return err;
 }
 
 static void acerhdf_unregister_platform(void)
@@ -589,7 +601,7 @@ static void acerhdf_unregister_platform(void)
 	if (!acerhdf_dev)
 		return;
 
-	platform_device_del(acerhdf_dev);
+	platform_device_unregister(acerhdf_dev);
 	platform_driver_unregister(&acerhdf_driver);
 }
 
@@ -633,7 +645,7 @@ static int __init acerhdf_init(void)
 
 	err = acerhdf_register_platform();
 	if (err)
-		goto err_unreg;
+		goto out_err;
 
 	err = acerhdf_register_thermal();
 	if (err)
@@ -646,7 +658,7 @@ err_unreg:
 	acerhdf_unregister_platform();
 
 out_err:
-	return -ENODEV;
+	return err;
 }
 
 static void __exit acerhdf_exit(void)
Signature · 1 line
1.5.4.3
#1 lore raw

From: Axel Lin <[email protected]>
Date: Wed, Jul 07, 2010 at 10:22:19AM +0800

Good, I like it. Go down for comments :)

Quoted 48 lines

This patch fix resource reclaim in below cases:
1. acerhdf_register_platform() does not properly handle platform_device_alloc() failure and platform_device_add() failure. This patch adds error handling for acerhdf_register_platform().
2. acerhdf_register_platform() return err with acerhdf_dev == NULL. as a result, acerhdf_unregister_platform() does not do resource reclaim in acerhdf_init() error path.
This patch adds error handing for acerhdf_register_platform(), thus correct the error handing path in acerhdf_init(). goto out_err instead of err_unreg if acerhdf_register_platform() fail.
3. platform_device_del() should be only used in error handling. Current implementation missed a platform_device_put() in acerhdf_exit. This patch fixes it by using platform_device_unregister() instead of platform_device_del() in acerhdf_unregister_platform().

---
 drivers/platform/x86/acerhdf.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)
drivers/platform/x86/acerhdf.c +13 -1
index 7b2384d..e938a86 100644
@@ -579,9 +579,21 @@ static int acerhdf_register_platform(void)
 		return err;
 
 	acerhdf_dev = platform_device_alloc("acerhdf", -1);
-	platform_device_add(acerhdf_dev);
+	if (!acerhdf_dev) {
+		err = -ENOMEM;
+		goto err_device_alloc;
+	}
+	err = platform_device_add(acerhdf_dev);
+	if (err)
+		goto err_device_add;
 
 	return 0;
+
+err_device_add:
+	platform_device_put(acerhdf_dev);
+err_device_alloc:
+	platform_driver_unregister(&acerhdf_driver);
+	return err;
 }
 
 static void acerhdf_unregister_platform(void)

drivers/platform/x86/acerhdf.c static void acerhdf_unregister_platform(void) · line 602

Quoted 2 lines
drivers/platform/x86/acerhdf.c +0 -0
@@ -589,7 +601,7 @@ static void acerhdf_unregister_platform(void)
 	if (!acerhdf_dev)
 		return;

while you're at it, you can remove the above check since platform_device_del() does that anyway and with your error checking, acerhdf_dev won't be unitialized in this path.

Quoted 29 lines
drivers/platform/x86/acerhdf.c +3 -3
-	platform_device_del(acerhdf_dev);
+	platform_device_unregister(acerhdf_dev);
 	platform_driver_unregister(&acerhdf_driver);
 }
 
@@ -633,7 +645,7 @@ static int __init acerhdf_init(void)
 
 	err = acerhdf_register_platform();
 	if (err)
-		goto err_unreg;
+		goto out_err;
 
 	err = acerhdf_register_thermal();
 	if (err)
@@ -646,7 +658,7 @@ err_unreg:
 	acerhdf_unregister_platform();
 
 out_err:
-	return -ENODEV;
+	return err;
 }
 
 static void __exit acerhdf_exit(void)
Signature · 1 line
1.5.4.3
Signature · 2 lines
Regards/Gruss,
    Boris.