[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <544AC56F16B56944AEC3BD4E3D591771324C0A9501@LIMKCMBX1.ad.analog.com>
Date: Tue, 25 Jan 2011 08:36:00 +0000
From: "Hennerich, Michael" <Michael.Hennerich@...log.com>
To: Julia Lawall <julia@...u.dk>, Paul Mundt <lethal@...ux-sh.org>
CC: "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
Mike Frysinger <vapier@...too.org>,
Bryan Wu <cooloney@...nel.org>,
"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/4] drivers/video/bf537-lq035.c: Add missing IS_ERR test
Julia Lawall wrote on 2011-01-24:
> lcd_device_register may return ERR_PTR, so a check is added for this
> value before the dereference. All of the other changes reorganize the
> error handling code in this function to avoid duplicating all of it in
> the added case.
>
> In the original code, in one case, the global variable fb_buffer was
> set to NULL in error code that appears after this variable is
> initialized. This is done now in all error handling code that has
> this property.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r@
> identifier f;
> @@
> f(...) { ... return ERR_PTR(...); }
>
> @@
> identifier r.f, fld;
> expression x;
> statement S1,S2;
> @@
> x = f(...) ... when != IS_ERR(x) ( if (IS_ERR(x) ||...) S1 else S2
> |
> *x->fld
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@...u.dk>
Looks good to me.
Acked-by: "Hennerich, Michael" <Michael.Hennerich@...log.com>
> ---
> drivers/video/bf537-lq035.c | 58 +++++++++++++++++++++++++----------
> --------- 1 file changed, 33 insertions(+), 25 deletions(-)
> diff --git a/drivers/video/bf537-lq035.c b/drivers/video/bf537-lq035.c
> index 18c5078..47c21fb 100644
> --- a/drivers/video/bf537-lq035.c
> +++ b/drivers/video/bf537-lq035.c
> @@ -696,6 +696,7 @@ static int __devinit bfin_lq035_probe(struct
> platform_device *pdev) {
> struct backlight_properties props;
> dma_addr_t dma_handle;
> + int ret;
>
> if (request_dma(CH_PPI, KBUILD_MODNAME)) {
> pr_err("couldn't request PPI DMA\n"); @@ -704,17 +705,16 @@ static
> int __devinit bfin_lq035_probe(struct platform_device *pdev)
>
> if (request_ports()) {
> pr_err("couldn't request gpio port\n");
> - free_dma(CH_PPI);
> - return -EFAULT;
> + ret = -EFAULT;
> + goto out_ports;
> }
>
> fb_buffer = dma_alloc_coherent(NULL, TOTAL_VIDEO_MEM_SIZE,
> &dma_handle, GFP_KERNEL);
> if (fb_buffer == NULL) {
> pr_err("couldn't allocate dma buffer\n");
> - free_dma(CH_PPI);
> - free_ports();
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto out_dma_coherent;
> }
>
> if (L1_DATA_A_LENGTH)
> @@ -725,10 +725,8 @@ static int __devinit bfin_lq035_probe(struct
> platform_device *pdev)
>
> if (dma_desc_table == NULL) {
> pr_err("couldn't allocate dma descriptor\n");
> - free_dma(CH_PPI);
> - free_ports();
> - dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer,
> 0);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto out_table;
> }
>
> bfin_lq035_fb.screen_base = (void *)fb_buffer; @@ -771,31 +769,21 @@
> static int __devinit bfin_lq035_probe(struct platform_device *pdev)
> bfin_lq035_fb.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
> if (bfin_lq035_fb.pseudo_palette == NULL) { pr_err("failed to
> allocate pseudo_palette\n");
> - free_dma(CH_PPI);
> - free_ports();
> - dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer,
> 0);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto out_palette;
> }
>
> if (fb_alloc_cmap(&bfin_lq035_fb.cmap, NBR_PALETTE, 0) < 0) {
> pr_err("failed to allocate colormap (%d entries)\n",
> NBR_PALETTE);
> - free_dma(CH_PPI);
> - free_ports();
> - dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer,
> 0);
> - kfree(bfin_lq035_fb.pseudo_palette);
> - return -EFAULT;
> + ret = -EFAULT;
> + goto out_cmap;
> }
>
> if (register_framebuffer(&bfin_lq035_fb) < 0) {
> pr_err("unable to register framebuffer\n");
> - free_dma(CH_PPI); - free_ports(); - dma_free_coherent(NULL,
> TOTAL_VIDEO_MEM_SIZE, fb_buffer, 0); - fb_buffer = NULL;
> - kfree(bfin_lq035_fb.pseudo_palette);
> - fb_dealloc_cmap(&bfin_lq035_fb.cmap); - return -EINVAL; + ret =
> -EINVAL; + goto out_reg;
> }
>
> i2c_add_driver(&ad5280_driver);
> @@ -807,11 +795,31 @@ static int __devinit bfin_lq035_probe(struct
> platform_device *pdev)
>
> lcd_dev = lcd_device_register(KBUILD_MODNAME, &pdev->dev, NULL,
> &bfin_lcd_ops);
> + if (IS_ERR(lcd_dev)) {
> + pr_err("unable to register lcd\n");
> + ret = PTR_ERR(lcd_dev);
> + goto out_lcd;
> + }
> lcd_dev->props.max_contrast = 255,
>
> pr_info("initialized");
>
> return 0;
> +out_lcd:
> + unregister_framebuffer(&bfin_lq035_fb);
> +out_reg:
> + fb_dealloc_cmap(&bfin_lq035_fb.cmap);
> +out_cmap:
> + kfree(bfin_lq035_fb.pseudo_palette);
> +out_palette:
> +out_table:
> + dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer, 0);
> + fb_buffer = NULL;
> +out_dma_coherent:
> + free_ports();
> +out_ports:
> + free_dma(CH_PPI);
> + return ret;
> }
>
> static int __devexit bfin_lq035_remove(struct platform_device *pdev)
Greetings,
Michael
--
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists