[PATCH] depca: fix leaks

[PATCH] depca: fix leaks

[Kulikov Vasiliy]

Since some of xxx_register_driver() can return error we must continue
with already registered drivers. If any of xxx_register_driver()
succeeded or depca_platform_probe() found any device then
depca_module_init() returns ok. In depca_module_exit() we must
unregister only registered drivers.

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 drivers/net/depca.c |   93 ++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index bf66e9b..4f43e12 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -551,6 +551,15 @@ static int io;
 static char *adapter_name;
 static int mem;			/* For loadable module assignment
 				   use insmod mem=0x????? .... */
+
+#ifdef CONFIG_MCA
+static int is_mca_registered;
+#endif
+#ifdef CONFIG_EISA
+static int is_eisa_registered;
+#endif
+static int is_platform_registered;
+
 module_param (irq, int, 0);
 module_param (io, int, 0);
 module_param (adapter_name, charp, 0);
@@ -1457,9 +1466,9 @@ static int __init depca_mca_probe(struct device *device)
 ** ISA bus I/O device probe
 */
 
-static void __init depca_platform_probe (void)
+static int __init depca_platform_probe(void)
 {
-	int i;
+	int i, n = 0;
 	struct platform_device *pldev;
 
 	for (i = 0; depca_io_ports[i].iobase; i++) {
@@ -1493,8 +1502,10 @@ static void __init depca_platform_probe (void)
 			depca_io_ports[i].device = NULL;
 			pldev->dev.platform_data = NULL;
 			platform_device_unregister (pldev);
-		}
+		} else
+			n++;
 	}
+	return n;
 }
 
 static enum depca_type __init depca_shmem_probe (ulong *mem_start)
@@ -2059,32 +2070,19 @@ static int depca_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	return status;
 }
 
-static int __init depca_module_init (void)
-{
-        int err = 0;
-
-#ifdef CONFIG_MCA
-        err = mca_register_driver (&depca_mca_driver);
-#endif
-#ifdef CONFIG_EISA
-        err |= eisa_driver_register (&depca_eisa_driver);
-#endif
-	err |= platform_driver_register (&depca_isa_driver);
-	depca_platform_probe ();
-
-        return err;
-}
-
-static void __exit depca_module_exit (void)
+static void depca_unregister(void)
 {
 	int i;
 #ifdef CONFIG_MCA
-        mca_unregister_driver (&depca_mca_driver);
+	if (is_mca_registered)
+		mca_unregister_driver(&depca_mca_driver);
 #endif
 #ifdef CONFIG_EISA
-        eisa_driver_unregister (&depca_eisa_driver);
+	if (is_eisa_registered)
+		eisa_driver_unregister(&depca_eisa_driver);
 #endif
-	platform_driver_unregister (&depca_isa_driver);
+	if (is_platform_registered)
+		platform_driver_unregister(&depca_isa_driver);
 
 	for (i = 0; depca_io_ports[i].iobase; i++) {
 		if (depca_io_ports[i].device) {
@@ -2095,5 +2093,54 @@ static void __exit depca_module_exit (void)
 	}
 }
 
+static int __init depca_module_init(void)
+{
+	int any = 0;
+	int platform_err = 0, err = 0;
+
+#ifdef CONFIG_MCA
+	{
+		int mca_err = 0;
+		mca_err = mca_register_driver(&depca_mca_driver);
+		if (!mca_err) {
+			any = 1;
+			is_mca_registered = 1;
+		} else
+			err = mca_err;
+	}
+#endif
+#ifdef CONFIG_EISA
+	{
+		int eisa_err = 0;
+		eisa_err = eisa_driver_register(&depca_eisa_driver);
+		if (!eisa_err) {
+			any = 1;
+			is_eisa_registered = 1;
+		} else
+			err = eisa_err;
+	}
+#endif
+	platform_err = platform_driver_register(&depca_isa_driver);
+	if (!platform_err) {
+		any = 1;
+		is_platform_registered = 1;
+	} else
+		err = platform_err;
+
+	if (depca_platform_probe())
+		any = 1;
+
+	if (any)
+		err = 0;
+	else if (err)
+		depca_unregister();
+	return err;
+}
+
+static void __exit depca_module_exit(void)
+{
+	depca_unregister();
+}
+
 module_init (depca_module_init);
 module_exit (depca_module_exit);
-- 
1.7.0.4

Re: [PATCH] depca: fix leaks

[David Miller]

From: Kulikov Vasiliy <segooon@gmail.com>
Date: Fri,  9 Jul 2010 16:36:25 +0400

> Since some of xxx_register_driver() can return error we must continue
> with already registered drivers. If any of xxx_register_driver()
> succeeded or depca_platform_probe() found any device then
> depca_module_init() returns ok. In depca_module_exit() we must
> unregister only registered drivers.
> 
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>

Driver registry should succeed except in cases that would indicate a
serious driver bug, such as trying to register the same driver twice.

Therefore if any registry fails, we should fail the entire probe.

This is the scheme used in just about any driver I've had any
involvement in, see for example drivers/net/niu.c:

--------------------
#ifdef CONFIG_SPARC64
	err = of_register_driver(&niu_of_driver, &of_bus_type);
#endif

	if (!err) {
		err = pci_register_driver(&niu_pci_driver);
#ifdef CONFIG_SPARC64
		if (err)
			of_unregister_driver(&niu_of_driver);
#endif
	}

	return err;
--------------------

Please do something similar here in depca, thanks.

[PATCH v2] depca: fix leaks in depca_module_init()

[Kulikov Vasiliy]

Since some of xxx_register_driver() can return error we must unregister
already registered drivers.

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 drivers/net/depca.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index bf66e9b..44c0694 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -2061,18 +2061,35 @@ static int depca_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 
 static int __init depca_module_init (void)
 {
-        int err = 0;
+	int err = 0;
 
 #ifdef CONFIG_MCA
-        err = mca_register_driver (&depca_mca_driver);
+	err = mca_register_driver(&depca_mca_driver);
+	if (err)
+		goto err;
 #endif
 #ifdef CONFIG_EISA
-        err |= eisa_driver_register (&depca_eisa_driver);
+	err = eisa_driver_register(&depca_eisa_driver);
+	if (err)
+		goto err_mca;
 #endif
-	err |= platform_driver_register (&depca_isa_driver);
-	depca_platform_probe ();
+	err = platform_driver_register(&depca_isa_driver);
+	if (err)
+		goto err_eisa;
 
-        return err;
+	depca_platform_probe();
+	return 0;
+
+err_eisa:
+#ifdef CONFIG_EISA
+	eisa_driver_unregister(&depca_eisa_driver);
+err_mca:
+#endif
+#ifdef CONFIG_MCA
+	mca_unregister_driver(&depca_mca_driver);
+err:
+#endif
+	return err;
 }
 
 static void __exit depca_module_exit (void)
-- 
1.7.0.4

Re: [PATCH v2] depca: fix leaks in depca_module_init()

[David Miller]

From: Kulikov Vasiliy <segooon@gmail.com>
Date: Mon, 12 Jul 2010 18:52:33 +0400

> Since some of xxx_register_driver() can return error we must unregister
> already registered drivers.
> 
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>

Applied.