[PATCH 5/15] skeletonfb: check fb_alloc_cmap return value and handle failure properly

[PATCH 5/15] skeletonfb: check fb_alloc_cmap return value and handle failure properly

[Andres Salomon]

Bad example code, no cookie!

Signed-off-by: Andres Salomon <dilinger@debian.org>
---
 drivers/video/skeletonfb.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
index df53365..a439159 100644
--- a/drivers/video/skeletonfb.c
+++ b/drivers/video/skeletonfb.c
@@ -795,8 +795,9 @@ static int __devinit xxxfb_probe(struct pci_dev *dev,
     if (!retval || retval == 4)
 	return -EINVAL;			
 
-    /* This has to been done !!! */	
-    fb_alloc_cmap(&info->cmap, cmap_len, 0);
+    /* This has to be done! */
+    if (fb_alloc_cmap(&info->cmap, cmap_len, 0))
+	return -ENOMEM;
 	
     /* 
      * The following is done in the case of having hardware with a static 
@@ -820,8 +821,10 @@ static int __devinit xxxfb_probe(struct pci_dev *dev,
      */
     /* xxxfb_set_par(info); */
 
-    if (register_framebuffer(info) < 0)
+    if (register_framebuffer(info) < 0) {
+	fb_dealloc_cmap(&info->cmap);
 	return -EINVAL;
+    }
     printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
 	   info->fix.id);
     pci_set_drvdata(dev, info); /* or platform_set_drvdata(pdev, info) */
-- 
1.5.6.5


------------------------------------------------------------------------------
Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM)
software. With Adobe AIR, Ajax developers can use existing skills and code to
build responsive, highly engaging applications that combine the power of local
resources and data with the reach of the web. Download the Adobe AIR SDK and
Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com

Re: [Linux-fbdev-devel] [PATCH 5/15] skeletonfb: check fb_alloc_cmap return value and handle failure properly

[Krzysztof Helt]

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

> 
> Bad example code, no cookie!
> 

It is even worse than you thought.

> Signed-off-by: Andres Salomon <dilinger@debian.org>
> ---
>  drivers/video/skeletonfb.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
> index df53365..a439159 100644
> --- a/drivers/video/skeletonfb.c
> +++ b/drivers/video/skeletonfb.c
> @@ -795,8 +795,9 @@ static int __devinit xxxfb_probe(struct pci_dev *dev,
>      if (!retval || retval == 4)
>  	return -EINVAL;			
>  
> -    /* This has to been done !!! */	
> -    fb_alloc_cmap(&info->cmap, cmap_len, 0);
> +    /* This has to be done! */
> +    if (fb_alloc_cmap(&info->cmap, cmap_len, 0))
> +	return -ENOMEM;
>  	

The info pointer should be freed before the return here ...

>      /* 
>       * The following is done in the case of having hardware with a static 
> @@ -820,8 +821,10 @@ static int __devinit xxxfb_probe(struct pci_dev *dev,
>       */
>      /* xxxfb_set_par(info); */
>  
> -    if (register_framebuffer(info) < 0)
> +    if (register_framebuffer(info) < 0) {
> +	fb_dealloc_cmap(&info->cmap);

... and here.

>  	return -EINVAL;
> +    }
>      printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
>  	   info->fix.id);
>      pci_set_drvdata(dev, info); /* or platform_set_drvdata(pdev, info) */
> -- 


Kind regards,
Krzysztof


----------------------------------------------------------------------
Zostan mistrzem parkowania w Bombaju!
Zagraj >> http://link.interia.pl/f204e 

Re: [Linux-fbdev-devel] [PATCH 5/15] skeletonfb: check fb_alloc_cmap return value and handle failure properly

[Andres Salomon]

On Mon, 9 Feb 2009 07:29:29 +0100
Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:

> On Sat, 7 Feb 2009 12:13:02 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > 
> > Bad example code, no cookie!
> > 
> 
> It is even worse than you thought.

Maybe skeletonfb.c should simply be dropped completely?  Incorrect
documentation is worse than no documentation.  If it's unmaintained,
buggy, and incorrect in many places...



> 
> > Signed-off-by: Andres Salomon <dilinger@debian.org>
> > ---
> >  drivers/video/skeletonfb.c |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
> > index df53365..a439159 100644
> > --- a/drivers/video/skeletonfb.c
> > +++ b/drivers/video/skeletonfb.c
> > @@ -795,8 +795,9 @@ static int __devinit xxxfb_probe(struct pci_dev
> > *dev, if (!retval || retval == 4)
> >  	return -EINVAL;			
> >  
> > -    /* This has to been done !!! */	
> > -    fb_alloc_cmap(&info->cmap, cmap_len, 0);
> > +    /* This has to be done! */
> > +    if (fb_alloc_cmap(&info->cmap, cmap_len, 0))
> > +	return -ENOMEM;
> >  	
> 
> The info pointer should be freed before the return here ...
> 
> >      /* 
> >       * The following is done in the case of having hardware with a
> > static @@ -820,8 +821,10 @@ static int __devinit xxxfb_probe(struct
> > pci_dev *dev, */
> >      /* xxxfb_set_par(info); */
> >  
> > -    if (register_framebuffer(info) < 0)
> > +    if (register_framebuffer(info) < 0) {
> > +	fb_dealloc_cmap(&info->cmap);
> 
> ... and here.
> 
> >  	return -EINVAL;
> > +    }
> >      printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
> >  	   info->fix.id);
> >      pci_set_drvdata(dev, info); /* or platform_set_drvdata(pdev,
> > info) */ -- 
> 
> 
> Kind regards,
> Krzysztof
> 
> 
> ----------------------------------------------------------------------
> Zostan mistrzem parkowania w Bombaju!
> Zagraj >> http://link.interia.pl/f204e 
>