All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 2/4] pxa168_eth: fix error handling in prope
@ 2010-08-24 16:53 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2010-08-24 16:53 UTC (permalink / raw)
  To: Sachin Sanap; +Cc: David S. Miller, netdev, kernel-janitors

A couple issues here:
* Some resources weren't released.  
* If alloc_etherdev() failed it would have caused a NULL dereference
  because "pep" would be null when we checked "if (pep->clk)".
* Also it's better to propagate the error codes from mdiobus_register()
  instead of just returning -ENOMEM.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c
index 857a681..302fb48 100644
--- a/drivers/net/pxa168_eth.c
+++ b/drivers/net/pxa168_eth.c
@@ -1497,7 +1497,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	dev = alloc_etherdev(sizeof(struct pxa168_eth_private));
 	if (!dev) {
 		err = -ENOMEM;
-		goto out;
+		goto err_clk;
 	}
 
 	platform_set_drvdata(pdev, dev);
@@ -1507,12 +1507,12 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
 		err = -ENODEV;
-		goto out;
+		goto err_netdev;
 	}
 	pep->base = ioremap(res->start, res->end - res->start + 1);
 	if (pep->base == NULL) {
 		err = -ENOMEM;
-		goto out;
+		goto err_netdev;
 	}
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	BUG_ON(!res);
@@ -1549,7 +1549,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	pep->smi_bus = mdiobus_alloc();
 	if (pep->smi_bus == NULL) {
 		err = -ENOMEM;
-		goto out;
+		goto err_base;
 	}
 	pep->smi_bus->priv = pep;
 	pep->smi_bus->name = "pxa168_eth smi";
@@ -1558,31 +1558,31 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	snprintf(pep->smi_bus->id, MII_BUS_ID_SIZE, "%d", pdev->id);
 	pep->smi_bus->parent = &pdev->dev;
 	pep->smi_bus->phy_mask = 0xffffffff;
-	if (mdiobus_register(pep->smi_bus) < 0) {
-		err = -ENOMEM;
-		goto out;
-	}
+	err = mdiobus_register(pep->smi_bus);
+	if (err)
+		goto err_free_mdio;
+
 	pxa168_init_hw(pep);
 	err = ethernet_phy_setup(dev);
 	if (err)
-		goto out;
+		goto err_mdiobus;
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	err = register_netdev(dev);
 	if (err)
-		goto out;
+		goto err_mdiobus;
 	return 0;
-out:
-	if (pep->clk) {
-		clk_disable(pep->clk);
-		clk_put(pep->clk);
-		pep->clk = NULL;
-	}
-	if (pep->base) {
-		iounmap(pep->base);
-		pep->base = NULL;
-	}
-	if (dev)
-		free_netdev(dev);
+
+err_mdiobus:
+	mdiobus_unregister(pep->smi_bus);
+err_free_mdio:
+	mdiobus_free(pep->smi_bus);
+err_base:
+	iounmap(pep->base);
+err_netdev:
+	free_netdev(dev);
+err_clk:
+	clk_disable(clk);
+	clk_put(clk);
 	return err;
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [patch 2/4] pxa168_eth: fix error handling in prope
@ 2010-08-24 16:53 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2010-08-24 16:53 UTC (permalink / raw)
  To: Sachin Sanap; +Cc: David S. Miller, netdev, kernel-janitors

A couple issues here:
* Some resources weren't released.  
* If alloc_etherdev() failed it would have caused a NULL dereference
  because "pep" would be null when we checked "if (pep->clk)".
* Also it's better to propagate the error codes from mdiobus_register()
  instead of just returning -ENOMEM.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c
index 857a681..302fb48 100644
--- a/drivers/net/pxa168_eth.c
+++ b/drivers/net/pxa168_eth.c
@@ -1497,7 +1497,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	dev = alloc_etherdev(sizeof(struct pxa168_eth_private));
 	if (!dev) {
 		err = -ENOMEM;
-		goto out;
+		goto err_clk;
 	}
 
 	platform_set_drvdata(pdev, dev);
@@ -1507,12 +1507,12 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res = NULL) {
 		err = -ENODEV;
-		goto out;
+		goto err_netdev;
 	}
 	pep->base = ioremap(res->start, res->end - res->start + 1);
 	if (pep->base = NULL) {
 		err = -ENOMEM;
-		goto out;
+		goto err_netdev;
 	}
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	BUG_ON(!res);
@@ -1549,7 +1549,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	pep->smi_bus = mdiobus_alloc();
 	if (pep->smi_bus = NULL) {
 		err = -ENOMEM;
-		goto out;
+		goto err_base;
 	}
 	pep->smi_bus->priv = pep;
 	pep->smi_bus->name = "pxa168_eth smi";
@@ -1558,31 +1558,31 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 	snprintf(pep->smi_bus->id, MII_BUS_ID_SIZE, "%d", pdev->id);
 	pep->smi_bus->parent = &pdev->dev;
 	pep->smi_bus->phy_mask = 0xffffffff;
-	if (mdiobus_register(pep->smi_bus) < 0) {
-		err = -ENOMEM;
-		goto out;
-	}
+	err = mdiobus_register(pep->smi_bus);
+	if (err)
+		goto err_free_mdio;
+
 	pxa168_init_hw(pep);
 	err = ethernet_phy_setup(dev);
 	if (err)
-		goto out;
+		goto err_mdiobus;
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	err = register_netdev(dev);
 	if (err)
-		goto out;
+		goto err_mdiobus;
 	return 0;
-out:
-	if (pep->clk) {
-		clk_disable(pep->clk);
-		clk_put(pep->clk);
-		pep->clk = NULL;
-	}
-	if (pep->base) {
-		iounmap(pep->base);
-		pep->base = NULL;
-	}
-	if (dev)
-		free_netdev(dev);
+
+err_mdiobus:
+	mdiobus_unregister(pep->smi_bus);
+err_free_mdio:
+	mdiobus_free(pep->smi_bus);
+err_base:
+	iounmap(pep->base);
+err_netdev:
+	free_netdev(dev);
+err_clk:
+	clk_disable(clk);
+	clk_put(clk);
 	return err;
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [patch 2/4] pxa168_eth: fix error handling in prope
  2010-08-24 16:53 ` Dan Carpenter
@ 2010-08-24 21:50   ` David Miller
  -1 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-08-24 21:50 UTC (permalink / raw)
  To: error27; +Cc: ssanap, netdev, kernel-janitors

From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:53:33 +0200

> A couple issues here:
> * Some resources weren't released.  
> * If alloc_etherdev() failed it would have caused a NULL dereference
>   because "pep" would be null when we checked "if (pep->clk)".
> * Also it's better to propagate the error codes from mdiobus_register()
>   instead of just returning -ENOMEM.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [patch 2/4] pxa168_eth: fix error handling in prope
@ 2010-08-24 21:50   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-08-24 21:50 UTC (permalink / raw)
  To: error27; +Cc: ssanap, netdev, kernel-janitors

From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:53:33 +0200

> A couple issues here:
> * Some resources weren't released.  
> * If alloc_etherdev() failed it would have caused a NULL dereference
>   because "pep" would be null when we checked "if (pep->clk)".
> * Also it's better to propagate the error codes from mdiobus_register()
>   instead of just returning -ENOMEM.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-08-24 21:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-24 16:53 [patch 2/4] pxa168_eth: fix error handling in prope Dan Carpenter
2010-08-24 16:53 ` Dan Carpenter
2010-08-24 21:50 ` David Miller
2010-08-24 21:50   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.