Messages in this thread | | | Date | Mon, 9 Feb 2009 17:32:00 -0500 | From | Andres Salomon <> | Subject | Re: [Linux-fbdev-devel] [PATCH 5/15] skeletonfb: check fb_alloc_cmap return value and handle failure properly |
| |
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 >
| |