[PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 == NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..5744af2 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -781,8 +781,10 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	if (!iocled)
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
-	if (!iocled->mmioaddr)
+	if (!iocled->mmioaddr) {
+		kfree(iocled);
 		return;
+	}
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -790,8 +792,11 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.label = "iocled";
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
-	if (gpiochip_add(&iocled->chip))
+	if (gpiochip_add(&iocled->chip)) {
+		iounmap(iocled->mmioaddr);
+		kfree(iocled);
 		return;
+	}
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 

[PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x = NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 = NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..5744af2 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -781,8 +781,10 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	if (!iocled)
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
-	if (!iocled->mmioaddr)
+	if (!iocled->mmioaddr) {
+		kfree(iocled);
 		return;
+	}
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -790,8 +792,11 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.label = "iocled";
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
-	if (gpiochip_add(&iocled->chip))
+	if (gpiochip_add(&iocled->chip)) {
+		iounmap(iocled->mmioaddr);
+		kfree(iocled);
 		return;
+	}
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Fri, 11 Sep 2009 18:21:00 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
> 
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)

Thank you for finding this out.

This patch add some correctness, but obviously incomplete: there are
more error pathes without iounmap/kfree/etc. in this function.

And I like goto-style cleanup, as Mark Brown said in reply for your
sound/soc patch.

Could you make a new patch?

---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Fri, 11 Sep 2009 18:21:00 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
> 
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)

Thank you for finding this out.

This patch add some correctness, but obviously incomplete: there are
more error pathes without iounmap/kfree/etc. in this function.

And I like goto-style cleanup, as Mark Brown said in reply for your
sound/soc patch.

Could you make a new patch?

---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Sun, 13 Sep 2009, Atsushi Nemoto wrote:

> On Fri, 11 Sep 2009 18:21:00 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
> > 
> > The semantic match that finds the first problem is as follows:
> > (http://www.emn.fr/x-info/coccinelle/)
> 
> Thank you for finding this out.
> 
> This patch add some correctness, but obviously incomplete: there are
> more error pathes without iounmap/kfree/etc. in this function.

The only other error path that I see is:

       pdev = platform_device_alloc("leds-gpio", basenum);
        if (!pdev)
                return;

But at that point the call gpiochip_add(&iocled->chip) has already 
succeeded.  From looking at this function, I have the impression that it 
makes the iocled structure available from a global array, gpio_desc.  
Since the function containing the above code doesn't return any error 
code, perhaps the caller will not know whether this platform_device_alloc 
error occurred or not.  There would also be at least the problem of 
getting the pointer out of the gpio_desc structure.  I guess this could be 
done with gpiochip_remove?

I can certainly make a new patch using the goto style, but let me know 
what to do about the above issue.

thanks,
julia


> And I like goto-style cleanup, as Mark Brown said in reply for your
> sound/soc patch.
> 
> Could you make a new patch?
> 
> ---
> Atsushi Nemoto
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Sun, 13 Sep 2009, Atsushi Nemoto wrote:

> On Fri, 11 Sep 2009 18:21:00 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
> > 
> > The semantic match that finds the first problem is as follows:
> > (http://www.emn.fr/x-info/coccinelle/)
> 
> Thank you for finding this out.
> 
> This patch add some correctness, but obviously incomplete: there are
> more error pathes without iounmap/kfree/etc. in this function.

The only other error path that I see is:

       pdev = platform_device_alloc("leds-gpio", basenum);
        if (!pdev)
                return;

But at that point the call gpiochip_add(&iocled->chip) has already 
succeeded.  From looking at this function, I have the impression that it 
makes the iocled structure available from a global array, gpio_desc.  
Since the function containing the above code doesn't return any error 
code, perhaps the caller will not know whether this platform_device_alloc 
error occurred or not.  There would also be at least the problem of 
getting the pointer out of the gpio_desc structure.  I guess this could be 
done with gpiochip_remove?

I can certainly make a new patch using the goto style, but let me know 
what to do about the above issue.

thanks,
julia


> And I like goto-style cleanup, as Mark Brown said in reply for your
> sound/soc patch.
> 
> Could you make a new patch?
> 
> ---
> Atsushi Nemoto
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Sun, 13 Sep 2009 17:14:06 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > This patch add some correctness, but obviously incomplete: there are
> > more error pathes without iounmap/kfree/etc. in this function.
> 
> The only other error path that I see is:
> 
>        pdev = platform_device_alloc("leds-gpio", basenum);
>         if (!pdev)
>                 return;
> 
> But at that point the call gpiochip_add(&iocled->chip) has already 
> succeeded.  From looking at this function, I have the impression that it 
> makes the iocled structure available from a global array, gpio_desc.  
> Since the function containing the above code doesn't return any error 
> code, perhaps the caller will not know whether this platform_device_alloc 
> error occurred or not.  There would also be at least the problem of 
> getting the pointer out of the gpio_desc structure.  I guess this could be 
> done with gpiochip_remove?
> 
> I can certainly make a new patch using the goto style, but let me know 
> what to do about the above issue.

Yes, this gpiochip is only used by leds-gpio driver.  So
gpiochip_remove() would be the right thing to do when something
failed.

Also there is one another error path: platform_device_add() failure at
the end of this function.

Thanks.
---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Sun, 13 Sep 2009 17:14:06 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > This patch add some correctness, but obviously incomplete: there are
> > more error pathes without iounmap/kfree/etc. in this function.
> 
> The only other error path that I see is:
> 
>        pdev = platform_device_alloc("leds-gpio", basenum);
>         if (!pdev)
>                 return;
> 
> But at that point the call gpiochip_add(&iocled->chip) has already 
> succeeded.  From looking at this function, I have the impression that it 
> makes the iocled structure available from a global array, gpio_desc.  
> Since the function containing the above code doesn't return any error 
> code, perhaps the caller will not know whether this platform_device_alloc 
> error occurred or not.  There would also be at least the problem of 
> getting the pointer out of the gpio_desc structure.  I guess this could be 
> done with gpiochip_remove?
> 
> I can certainly make a new patch using the goto style, but let me know 
> what to do about the above issue.

Yes, this gpiochip is only used by leds-gpio driver.  So
gpiochip_remove() would be the right thing to do when something
failed.

Also there is one another error path: platform_device_add() failure at
the end of this function.

Thanks.
---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Mon, 14 Sep 2009, Atsushi Nemoto wrote:

> On Sun, 13 Sep 2009 17:14:06 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > > This patch add some correctness, but obviously incomplete: there are
> > > more error pathes without iounmap/kfree/etc. in this function.
> > 
> > The only other error path that I see is:
> > 
> >        pdev = platform_device_alloc("leds-gpio", basenum);
> >         if (!pdev)
> >                 return;
> > 
> > But at that point the call gpiochip_add(&iocled->chip) has already 
> > succeeded.  From looking at this function, I have the impression that it 
> > makes the iocled structure available from a global array, gpio_desc.  
> > Since the function containing the above code doesn't return any error 
> > code, perhaps the caller will not know whether this platform_device_alloc 
> > error occurred or not.  There would also be at least the problem of 
> > getting the pointer out of the gpio_desc structure.  I guess this could be 
> > done with gpiochip_remove?
> > 
> > I can certainly make a new patch using the goto style, but let me know 
> > what to do about the above issue.
> 
> Yes, this gpiochip is only used by leds-gpio driver.  So
> gpiochip_remove() would be the right thing to do when something
> failed.
> 
> Also there is one another error path: platform_device_add() failure at
> the end of this function.

OK, I see.  I will submit an improved patch.  Thanks for the explanations.

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Mon, 14 Sep 2009, Atsushi Nemoto wrote:

> On Sun, 13 Sep 2009 17:14:06 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > > This patch add some correctness, but obviously incomplete: there are
> > > more error pathes without iounmap/kfree/etc. in this function.
> > 
> > The only other error path that I see is:
> > 
> >        pdev = platform_device_alloc("leds-gpio", basenum);
> >         if (!pdev)
> >                 return;
> > 
> > But at that point the call gpiochip_add(&iocled->chip) has already 
> > succeeded.  From looking at this function, I have the impression that it 
> > makes the iocled structure available from a global array, gpio_desc.  
> > Since the function containing the above code doesn't return any error 
> > code, perhaps the caller will not know whether this platform_device_alloc 
> > error occurred or not.  There would also be at least the problem of 
> > getting the pointer out of the gpio_desc structure.  I guess this could be 
> > done with gpiochip_remove?
> > 
> > I can certainly make a new patch using the goto style, but let me know 
> > what to do about the above issue.
> 
> Yes, this gpiochip is only used by leds-gpio driver.  So
> gpiochip_remove() would be the right thing to do when something
> failed.
> 
> Also there is one another error path: platform_device_add() failure at
> the end of this function.

OK, I see.  I will submit an improved patch.  Thanks for the explanations.

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 == NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..c860810 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
 	if (!iocled->mmioaddr)
-		return;
+		goto out_free;
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
 	if (gpiochip_add(&iocled->chip))
-		return;
+		goto out_unmap;
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 
 	pdev = platform_device_alloc("leds-gpio", basenum);
 	if (!pdev)
-		return;
+		goto out_gpio;
 	iocled->pdata.num_leds = num;
 	iocled->pdata.leds = iocled->leds;
 	for (i = 0; i < num; i++) {
@@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	}
 	pdev->dev.platform_data = &iocled->pdata;
 	if (platform_device_add(pdev))
-		platform_device_put(pdev);
+		goto out_pdev;
+	return;
+out_pdev:
+	platform_device_put(pdev);
+out_gpio:
+	gpio_remove(&iocled->chip);
+out_unmap:
+	iounmap(iocled->mmioaddr);
+out_free:
+	kfree(iocled);
 }
 #else /* CONFIG_LEDS_GPIO */
 void __init txx9_iocled_init(unsigned long baseaddr,

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x = NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 = NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..c860810 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
 	if (!iocled->mmioaddr)
-		return;
+		goto out_free;
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
 	if (gpiochip_add(&iocled->chip))
-		return;
+		goto out_unmap;
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 
 	pdev = platform_device_alloc("leds-gpio", basenum);
 	if (!pdev)
-		return;
+		goto out_gpio;
 	iocled->pdata.num_leds = num;
 	iocled->pdata.leds = iocled->leds;
 	for (i = 0; i < num; i++) {
@@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	}
 	pdev->dev.platform_data = &iocled->pdata;
 	if (platform_device_add(pdev))
-		platform_device_put(pdev);
+		goto out_pdev;
+	return;
+out_pdev:
+	platform_device_put(pdev);
+out_gpio:
+	gpio_remove(&iocled->chip);
+out_unmap:
+	iounmap(iocled->mmioaddr);
+out_free:
+	kfree(iocled);
 }
 #else /* CONFIG_LEDS_GPIO */
 void __init txx9_iocled_init(unsigned long baseaddr,

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Ralf Baechle]

On Sun, Sep 13, 2009 at 09:15:18PM +0200, Julia Lawall wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
> 
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)

Guess this one looks right, applied.

Thanks,

  Ralf

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Ralf Baechle]

On Sun, Sep 13, 2009 at 09:15:18PM +0200, Julia Lawall wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
> 
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)

Guess this one looks right, applied.

Thanks,

  Ralf

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

> +out_pdev:
> +	platform_device_put(pdev);
> +out_gpio:
> +	gpio_remove(&iocled->chip);

I just noticed that the prototype of gpio_remove has __must_check
I don't think there is anything to check here; since the thing is not 
fully initialized here, it is unlikely to be busy.  Should there be (void) 
in front of it?

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

> +out_pdev:
> +	platform_device_put(pdev);
> +out_gpio:
> +	gpio_remove(&iocled->chip);

I just noticed that the prototype of gpio_remove has __must_check
I don't think there is anything to check here; since the thing is not 
fully initialized here, it is unlikely to be busy.  Should there be (void) 
in front of it?

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Ralf Rösch]

The gpio_remove is missing:
  arch/mips/txx9/generic/setup.c:838: error: implicit declaration of function ‘gpio_remove’

Should it be gpiochip_remove() instead?

--
Ralf Roesch


The Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
>
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
>
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> statement S;
> expression E;
> identifier f,f1,l;
> position p1,p2;
> expression *ptr != NULL;
> @@
>
> x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
> ...
> if (x == NULL) S
> <... when != x
>      when != if (...) { <+...x...+> }
> (
> x->f1 = E
> |
>  (x->f1 == NULL || ...)
> |
>  f(...,x->f1,...)
> )
> ...>
> (
>  return \(0\|<+...x...+>\|ptr\);
> |
>  return@p2 ...;
> )
>
> @script:python@
> p1 << r.p1;
> p2 << r.p2;
> @@
>
> print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
> ---
>  arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> index a205e2b..c860810 100644
> --- a/arch/mips/txx9/generic/setup.c
> +++ b/arch/mips/txx9/generic/setup.c
> @@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  		return;
>  	iocled->mmioaddr = ioremap(baseaddr, 1);
>  	if (!iocled->mmioaddr)
> -		return;
> +		goto out_free;
>  	iocled->chip.get = txx9_iocled_get;
>  	iocled->chip.set = txx9_iocled_set;
>  	iocled->chip.direction_input = txx9_iocled_dir_in;
> @@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  	iocled->chip.base = basenum;
>  	iocled->chip.ngpio = num;
>  	if (gpiochip_add(&iocled->chip))
> -		return;
> +		goto out_unmap;
>  	if (basenum < 0)
>  		basenum = iocled->chip.base;
>  
>  	pdev = platform_device_alloc("leds-gpio", basenum);
>  	if (!pdev)
> -		return;
> +		goto out_gpio;
>  	iocled->pdata.num_leds = num;
>  	iocled->pdata.leds = iocled->leds;
>  	for (i = 0; i < num; i++) {
> @@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  	}
>  	pdev->dev.platform_data = &iocled->pdata;
>  	if (platform_device_add(pdev))
> -		platform_device_put(pdev);
> +		goto out_pdev;
> +	return;
> +out_pdev:
> +	platform_device_put(pdev);
> +out_gpio:
> +	gpio_remove(&iocled->chip);
> +out_unmap:
> +	iounmap(iocled->mmioaddr);
> +out_free:
> +	kfree(iocled);
>  }
>  #else /* CONFIG_LEDS_GPIO */
>  void __init txx9_iocled_init(unsigned long baseaddr,
>
>   

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Ralf Rösch]

The gpio_remove is missing:
  arch/mips/txx9/generic/setup.c:838: error: implicit declaration of function ‘gpio_remove’

Should it be gpiochip_remove() instead?

--
Ralf Roesch


The Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
>
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
>
> The semantic match that finds the first problem is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
>
> // <smpl>
> @r exists@
> local idexpression x;
> statement S;
> expression E;
> identifier f,f1,l;
> position p1,p2;
> expression *ptr != NULL;
> @@
>
> x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
> ...
> if (x = NULL) S
> <... when != x
>      when != if (...) { <+...x...+> }
> (
> x->f1 = E
> |
>  (x->f1 = NULL || ...)
> |
>  f(...,x->f1,...)
> )
> ...>
> (
>  return \(0\|<+...x...+>\|ptr\);
> |
>  return@p2 ...;
> )
>
> @script:python@
> p1 << r.p1;
> p2 << r.p2;
> @@
>
> print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
> ---
>  arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> index a205e2b..c860810 100644
> --- a/arch/mips/txx9/generic/setup.c
> +++ b/arch/mips/txx9/generic/setup.c
> @@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  		return;
>  	iocled->mmioaddr = ioremap(baseaddr, 1);
>  	if (!iocled->mmioaddr)
> -		return;
> +		goto out_free;
>  	iocled->chip.get = txx9_iocled_get;
>  	iocled->chip.set = txx9_iocled_set;
>  	iocled->chip.direction_input = txx9_iocled_dir_in;
> @@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  	iocled->chip.base = basenum;
>  	iocled->chip.ngpio = num;
>  	if (gpiochip_add(&iocled->chip))
> -		return;
> +		goto out_unmap;
>  	if (basenum < 0)
>  		basenum = iocled->chip.base;
>  
>  	pdev = platform_device_alloc("leds-gpio", basenum);
>  	if (!pdev)
> -		return;
> +		goto out_gpio;
>  	iocled->pdata.num_leds = num;
>  	iocled->pdata.leds = iocled->leds;
>  	for (i = 0; i < num; i++) {
> @@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
>  	}
>  	pdev->dev.platform_data = &iocled->pdata;
>  	if (platform_device_add(pdev))
> -		platform_device_put(pdev);
> +		goto out_pdev;
> +	return;
> +out_pdev:
> +	platform_device_put(pdev);
> +out_gpio:
> +	gpio_remove(&iocled->chip);
> +out_unmap:
> +	iounmap(iocled->mmioaddr);
> +out_free:
> +	kfree(iocled);
>  }
>  #else /* CONFIG_LEDS_GPIO */
>  void __init txx9_iocled_init(unsigned long baseaddr,
>
>   

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3642 bytes --]

On Tue, 15 Sep 2009, Ralf Rösch wrote:

> The gpio_remove is missing:
>  arch/mips/txx9/generic/setup.c:838: error: implicit declaration of function
>  ?gpio_remove?
> 
> Should it be gpiochip_remove() instead?

Oops, sorry about that.  That is indeed the function I was looking at.

I can submit a revised patch, but I sent another question asking about 
whether something should be done about the fact that gpiochip_remove is 
declared as follows:

extern int __must_check gpiochip_remove(struct gpio_chip *chip);

I don't think the call can fail at this point, but should something be 
done to avoid a compiler warning?

thanks,
julia


> --
> Ralf Roesch
> 
> 
> The Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
> >
> > The semantic match that finds the first problem is as follows:
> > (http://www.emn.fr/x-info/coccinelle/)
> >
> > // <smpl>
> > @r exists@
> > local idexpression x;
> > statement S;
> > expression E;
> > identifier f,f1,l;
> > position p1,p2;
> > expression *ptr != NULL;
> > @@
> >
> > x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
> > ...
> > if (x == NULL) S
> > <... when != x
> >      when != if (...) { <+...x...+> }
> > (
> > x->f1 = E
> > |
> >  (x->f1 == NULL || ...)
> > |
> >  f(...,x->f1,...)
> > )
> > ...>
> > (
> >  return \(0\|<+...x...+>\|ptr\);
> > |
> >  return@p2 ...;
> > )
> >
> > @script:python@
> > p1 << r.p1;
> > p2 << r.p2;
> > @@
> >
> > print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > ---
> >  arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
> >  1 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> > index a205e2b..c860810 100644
> > --- a/arch/mips/txx9/generic/setup.c
> > +++ b/arch/mips/txx9/generic/setup.c
> > @@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   	return;
> >   iocled->mmioaddr = ioremap(baseaddr, 1);
> >   if (!iocled->mmioaddr)
> > -		return;
> > +		goto out_free;
> >   iocled->chip.get = txx9_iocled_get;
> >   iocled->chip.set = txx9_iocled_set;
> >   iocled->chip.direction_input = txx9_iocled_dir_in;
> > @@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   iocled->chip.base = basenum;
> >   iocled->chip.ngpio = num;
> >   if (gpiochip_add(&iocled->chip))
> > -		return;
> > +		goto out_unmap;
> >   if (basenum < 0)
> >    basenum = iocled->chip.base;
> >  
> >   pdev = platform_device_alloc("leds-gpio", basenum);
> >   if (!pdev)
> > -		return;
> > +		goto out_gpio;
> >   iocled->pdata.num_leds = num;
> >   iocled->pdata.leds = iocled->leds;
> >   for (i = 0; i < num; i++) {
> > @@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   }
> >   pdev->dev.platform_data = &iocled->pdata;
> >   if (platform_device_add(pdev))
> > -		platform_device_put(pdev);
> > +		goto out_pdev;
> > +	return;
> > +out_pdev:
> > +	platform_device_put(pdev);
> > +out_gpio:
> > +	gpio_remove(&iocled->chip);
> > +out_unmap:
> > +	iounmap(iocled->mmioaddr);
> > +out_free:
> > +	kfree(iocled);
> >  }
> >  #else /* CONFIG_LEDS_GPIO */
> >  void __init txx9_iocled_init(unsigned long baseaddr,
> >
> >   
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3775 bytes --]

On Tue, 15 Sep 2009, Ralf Rösch wrote:

> The gpio_remove is missing:
>  arch/mips/txx9/generic/setup.c:838: error: implicit declaration of function
>  ?gpio_remove?
> 
> Should it be gpiochip_remove() instead?

Oops, sorry about that.  That is indeed the function I was looking at.

I can submit a revised patch, but I sent another question asking about 
whether something should be done about the fact that gpiochip_remove is 
declared as follows:

extern int __must_check gpiochip_remove(struct gpio_chip *chip);

I don't think the call can fail at this point, but should something be 
done to avoid a compiler warning?

thanks,
julia


> --
> Ralf Roesch
> 
> 
> The Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
> >
> > The semantic match that finds the first problem is as follows:
> > (http://www.emn.fr/x-info/coccinelle/)
> >
> > // <smpl>
> > @r exists@
> > local idexpression x;
> > statement S;
> > expression E;
> > identifier f,f1,l;
> > position p1,p2;
> > expression *ptr != NULL;
> > @@
> >
> > x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
> > ...
> > if (x == NULL) S
> > <... when != x
> >      when != if (...) { <+...x...+> }
> > (
> > x->f1 = E
> > |
> >  (x->f1 == NULL || ...)
> > |
> >  f(...,x->f1,...)
> > )
> > ...>
> > (
> >  return \(0\|<+...x...+>\|ptr\);
> > |
> >  return@p2 ...;
> > )
> >
> > @script:python@
> > p1 << r.p1;
> > p2 << r.p2;
> > @@
> >
> > print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > ---
> >  arch/mips/txx9/generic/setup.c      |   17 +++++++++++++----
> >  1 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> > index a205e2b..c860810 100644
> > --- a/arch/mips/txx9/generic/setup.c
> > +++ b/arch/mips/txx9/generic/setup.c
> > @@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   	return;
> >   iocled->mmioaddr = ioremap(baseaddr, 1);
> >   if (!iocled->mmioaddr)
> > -		return;
> > +		goto out_free;
> >   iocled->chip.get = txx9_iocled_get;
> >   iocled->chip.set = txx9_iocled_set;
> >   iocled->chip.direction_input = txx9_iocled_dir_in;
> > @@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   iocled->chip.base = basenum;
> >   iocled->chip.ngpio = num;
> >   if (gpiochip_add(&iocled->chip))
> > -		return;
> > +		goto out_unmap;
> >   if (basenum < 0)
> >    basenum = iocled->chip.base;
> >  
> >   pdev = platform_device_alloc("leds-gpio", basenum);
> >   if (!pdev)
> > -		return;
> > +		goto out_gpio;
> >   iocled->pdata.num_leds = num;
> >   iocled->pdata.leds = iocled->leds;
> >   for (i = 0; i < num; i++) {
> > @@ -812,7 +812,16 @@ void __init txx9_iocled_init(unsigned long baseaddr,
> >   }
> >   pdev->dev.platform_data = &iocled->pdata;
> >   if (platform_device_add(pdev))
> > -		platform_device_put(pdev);
> > +		goto out_pdev;
> > +	return;
> > +out_pdev:
> > +	platform_device_put(pdev);
> > +out_gpio:
> > +	gpio_remove(&iocled->chip);
> > +out_unmap:
> > +	iounmap(iocled->mmioaddr);
> > +out_free:
> > +	kfree(iocled);
> >  }
> >  #else /* CONFIG_LEDS_GPIO */
> >  void __init txx9_iocled_init(unsigned long baseaddr,
> >
> >   
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Tue, 15 Sep 2009 07:03:42 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > +out_pdev:
> > +	platform_device_put(pdev);
> > +out_gpio:
> > +	gpio_remove(&iocled->chip);
> 
> I just noticed that the prototype of gpio_remove has __must_check
> I don't think there is anything to check here; since the thing is not 
> fully initialized here, it is unlikely to be busy.  Should there be (void) 
> in front of it?

The void casting does not silence the warning.  How about this?

	if (gpiochip_remove(&iocled->chip))
		return;

In general, memory leak would be less serious than crash or data
corruption ;)

---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Atsushi Nemoto]

On Tue, 15 Sep 2009 07:03:42 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > +out_pdev:
> > +	platform_device_put(pdev);
> > +out_gpio:
> > +	gpio_remove(&iocled->chip);
> 
> I just noticed that the prototype of gpio_remove has __must_check
> I don't think there is anything to check here; since the thing is not 
> fully initialized here, it is unlikely to be busy.  Should there be (void) 
> in front of it?

The void casting does not silence the warning.  How about this?

	if (gpiochip_remove(&iocled->chip))
		return;

In general, memory leak would be less serious than crash or data
corruption ;)

---
Atsushi Nemoto

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Wed, 16 Sep 2009, Atsushi Nemoto wrote:

> On Tue, 15 Sep 2009 07:03:42 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > > +out_pdev:
> > > +	platform_device_put(pdev);
> > > +out_gpio:
> > > +	gpio_remove(&iocled->chip);
> > 
> > I just noticed that the prototype of gpio_remove has __must_check
> > I don't think there is anything to check here; since the thing is not 
> > fully initialized here, it is unlikely to be busy.  Should there be (void) 
> > in front of it?
> 
> The void casting does not silence the warning.  How about this?
> 
> 	if (gpiochip_remove(&iocled->chip))
> 		return;
> 
> In general, memory leak would be less serious than crash or data
> corruption ;)

OK, I will send an updated patch shortly.

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

On Wed, 16 Sep 2009, Atsushi Nemoto wrote:

> On Tue, 15 Sep 2009 07:03:42 +0200 (CEST), Julia Lawall <julia@diku.dk> wrote:
> > > +out_pdev:
> > > +	platform_device_put(pdev);
> > > +out_gpio:
> > > +	gpio_remove(&iocled->chip);
> > 
> > I just noticed that the prototype of gpio_remove has __must_check
> > I don't think there is anything to check here; since the thing is not 
> > fully initialized here, it is unlikely to be busy.  Should there be (void) 
> > in front of it?
> 
> The void casting does not silence the warning.  How about this?
> 
> 	if (gpiochip_remove(&iocled->chip))
> 		return;
> 
> In general, memory leak would be less serious than crash or data
> corruption ;)

OK, I will send an updated patch shortly.

julia

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 == NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..dfe4720 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
 	if (!iocled->mmioaddr)
-		return;
+		goto out_free;
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
 	if (gpiochip_add(&iocled->chip))
-		return;
+		goto out_unmap;
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 
 	pdev = platform_device_alloc("leds-gpio", basenum);
 	if (!pdev)
-		return;
+		goto out_gpio;
 	iocled->pdata.num_leds = num;
 	iocled->pdata.leds = iocled->leds;
 	for (i = 0; i < num; i++) {
@@ -812,7 +812,17 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	}
 	pdev->dev.platform_data = &iocled->pdata;
 	if (platform_device_add(pdev))
-		platform_device_put(pdev);
+		goto out_pdev;
+	return;
+out_pdev:
+	platform_device_put(pdev);
+out_gpio:
+	if (gpiochip_remove(&iocled->chip))
+		return;
+out_unmap:
+	iounmap(iocled->mmioaddr);
+out_free:
+	kfree(iocled);
 }
 #else /* CONFIG_LEDS_GPIO */
 void __init txx9_iocled_init(unsigned long baseaddr,

Re: [PATCH 1/8] arch/mips/txx9: introduce missing kfree, iounmap

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.

The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x = NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 = NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 arch/mips/txx9/generic/setup.c      |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index a205e2b..dfe4720 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -782,7 +782,7 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 		return;
 	iocled->mmioaddr = ioremap(baseaddr, 1);
 	if (!iocled->mmioaddr)
-		return;
+		goto out_free;
 	iocled->chip.get = txx9_iocled_get;
 	iocled->chip.set = txx9_iocled_set;
 	iocled->chip.direction_input = txx9_iocled_dir_in;
@@ -791,13 +791,13 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	iocled->chip.base = basenum;
 	iocled->chip.ngpio = num;
 	if (gpiochip_add(&iocled->chip))
-		return;
+		goto out_unmap;
 	if (basenum < 0)
 		basenum = iocled->chip.base;
 
 	pdev = platform_device_alloc("leds-gpio", basenum);
 	if (!pdev)
-		return;
+		goto out_gpio;
 	iocled->pdata.num_leds = num;
 	iocled->pdata.leds = iocled->leds;
 	for (i = 0; i < num; i++) {
@@ -812,7 +812,17 @@ void __init txx9_iocled_init(unsigned long baseaddr,
 	}
 	pdev->dev.platform_data = &iocled->pdata;
 	if (platform_device_add(pdev))
-		platform_device_put(pdev);
+		goto out_pdev;
+	return;
+out_pdev:
+	platform_device_put(pdev);
+out_gpio:
+	if (gpiochip_remove(&iocled->chip))
+		return;
+out_unmap:
+	iounmap(iocled->mmioaddr);
+out_free:
+	kfree(iocled);
 }
 #else /* CONFIG_LEDS_GPIO */
 void __init txx9_iocled_init(unsigned long baseaddr,