[PATCH 13/15] amba-clcd: fix cmap memory leaks

[PATCH 13/15] amba-clcd: fix cmap memory leaks

[Andres Salomon]

 - fix cmap leak in removal path
 - fix cmap leak when register_framebuffer fails

Signed-off-by: Andres Salomon <dilinger@debian.org>
---
 drivers/video/amba-clcd.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index 4e046fe..61050ab 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -408,7 +408,9 @@ static int clcdfb_register(struct clcd_fb *fb)
 	/*
 	 * Allocate colourmap.
 	 */
-	fb_alloc_cmap(&fb->fb.cmap, 256, 0);
+	ret = fb_alloc_cmap(&fb->fb.cmap, 256, 0);
+	if (ret)
+		goto unmap;
 
 	/*
 	 * Ensure interrupts are disabled.
@@ -426,6 +428,8 @@ static int clcdfb_register(struct clcd_fb *fb)
 
 	printk(KERN_ERR "CLCD: cannot register framebuffer (%d)\n", ret);
 
+	fb_dealloc_cmap(&fb->fb.cmap);
+ unmap:
 	iounmap(fb->regs);
  free_clk:
 	clk_put(fb->clk);
@@ -485,6 +489,8 @@ static int clcdfb_remove(struct amba_device *dev)
 
 	clcdfb_disable(fb);
 	unregister_framebuffer(&fb->fb);
+	if (fb->fb.cmap.len)
+		fb_dealloc_cmap(&fb->fb.cmap);
 	iounmap(fb->regs);
 	clk_put(fb->clk);
 
-- 
1.5.6.5

Re: [Linux-fbdev-devel] [PATCH 13/15] amba-clcd: fix cmap memory leaks

[Krzysztof Helt]

On Sat, 7 Feb 2009 12:18:48 -0500
Andres Salomon <dilinger@queued.net> wrote:

> 
>  - fix cmap leak in removal path
>  - fix cmap leak when register_framebuffer fails
> 
> Signed-off-by: Andres Salomon <dilinger@debian.org>
> ---

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

One comment (see below).

>  drivers/video/amba-clcd.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
> index 4e046fe..61050ab 100644
> --- a/drivers/video/amba-clcd.c
> +++ b/drivers/video/amba-clcd.c
> @@ -408,7 +408,9 @@ static int clcdfb_register(struct clcd_fb *fb)
>  	/*
>  	 * Allocate colourmap.
>  	 */
> -	fb_alloc_cmap(&fb->fb.cmap, 256, 0);
> +	ret = fb_alloc_cmap(&fb->fb.cmap, 256, 0);
> +	if (ret)
> +		goto unmap;
>  
>  	/*
>  	 * Ensure interrupts are disabled.
> @@ -426,6 +428,8 @@ static int clcdfb_register(struct clcd_fb *fb)
>  
>  	printk(KERN_ERR "CLCD: cannot register framebuffer (%d)\n", ret);
>  
> +	fb_dealloc_cmap(&fb->fb.cmap);
> + unmap:
>  	iounmap(fb->regs);
>   free_clk:
>  	clk_put(fb->clk);
> @@ -485,6 +489,8 @@ static int clcdfb_remove(struct amba_device *dev)
>  
>  	clcdfb_disable(fb);
>  	unregister_framebuffer(&fb->fb);
> +	if (fb->fb.cmap.len)
> +		fb_dealloc_cmap(&fb->fb.cmap);

Is this if() needed? Is this function called twice (or more) or
can it be called without the cmap allocated?

>  	iounmap(fb->regs);
>  	clk_put(fb->clk);
>  
> -- 
> 1.5.6.5
> 

Re: [Linux-fbdev-devel] [PATCH 13/15] amba-clcd: fix cmap memory leaks

[Andres Salomon]

On Mon, 9 Feb 2009 21:16:14 +0100
Krzysztof Helt <krzysztof.h1@wp.pl> wrote:

> On Sat, 7 Feb 2009 12:18:48 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > 
> >  - fix cmap leak in removal path
> >  - fix cmap leak when register_framebuffer fails
> > 
> > Signed-off-by: Andres Salomon <dilinger@debian.org>
> > ---
> 
> Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> One comment (see below).
> 
[...]
> > @@ -485,6 +489,8 @@ static int clcdfb_remove(struct amba_device
> > *dev) 
> >  	clcdfb_disable(fb);
> >  	unregister_framebuffer(&fb->fb);
> > +	if (fb->fb.cmap.len)
> > +		fb_dealloc_cmap(&fb->fb.cmap);
> 
> Is this if() needed? Is this function called twice (or more) or
> can it be called without the cmap allocated?


It was added as a precaution due to the fb->board->remove callbacks,
but I think I misunderstood the code.  So my official answer is "I'm not
sure." :)





> 
> >  	iounmap(fb->regs);
> >  	clk_put(fb->clk);
> >  
> > -- 
> > 1.5.6.5
> >