[PATCH 0/9] video: fbdev: Deletion of some unnecessary checks

STALE3412d

10 messages, 1 author, 2015-01-19 · open the first message on its own page

[PATCH 0/9] video: fbdev: Deletion of some unnecessary checks

From: SF Markus Elfring <hidden>
Date: 2015-01-19 17:55:15

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 17:57:25 +0100

Further update suggestions were taken into account after a patch was applied
from static source code analysis.

Markus Elfring (9):
  fbdev: Deletion of checks before the function call "iounmap"
  One function call less in tdfxfb_probe() after error detection
  One function call less in vesafb_probe() after error detection
  Add check for return value of ioremap() in cirrusfb_pci_register()
  Less function calls in trident_pci_probe() after error detection
  One function call less in pvr2fb_common_init() after error detection
  One function call less in tgafb_register() after error detection
  Less function calls in valkyriefb_init() after error detection
  Less function calls in w100fb_probe() after error detection

 drivers/video/fbdev/aty/atyfb_base.c   | 15 +++++--------
 drivers/video/fbdev/cirrusfb.c         | 10 ++++++---
 drivers/video/fbdev/controlfb.c        |  6 ++---
 drivers/video/fbdev/efifb.c            |  3 +--
 drivers/video/fbdev/hgafb.c            |  3 +--
 drivers/video/fbdev/hpfb.c             |  3 +--
 drivers/video/fbdev/i810/i810_main.c   |  6 ++---
 drivers/video/fbdev/macfb.c            | 15 +++++--------
 drivers/video/fbdev/offb.c             |  3 +--
 drivers/video/fbdev/pvr2fb.c           | 15 ++++++-------
 drivers/video/fbdev/s1d13xxxfb.c       |  3 +--
 drivers/video/fbdev/s3fb.c             |  6 ++---
 drivers/video/fbdev/sh_mobile_hdmi.c   |  6 ++---
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  3 +--
 drivers/video/fbdev/simplefb.c         |  3 +--
 drivers/video/fbdev/stifb.c            |  3 +--
 drivers/video/fbdev/tdfxfb.c           | 22 +++++++++----------
 drivers/video/fbdev/tgafb.c            | 20 ++++++++---------
 drivers/video/fbdev/tridentfb.c        | 15 +++++++------
 drivers/video/fbdev/valkyriefb.c       | 37 ++++++++++++++++++-------------
 drivers/video/fbdev/vesafb.c           | 11 +++++-----
 drivers/video/fbdev/w100fb.c           | 40 +++++++++++++++++-----------------
 22 files changed, 116 insertions(+), 132 deletions(-)

-- 
2.2.2

[PATCH 1/9] video: Deletion of checks before the function call "iounmap"

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:00:39

From: Markus Elfring <redacted>
Date: Sun, 18 Jan 2015 22:45:39 +0100

The iounmap() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/aty/atyfb_base.c   | 15 +++++----------
 drivers/video/fbdev/cirrusfb.c         |  3 +--
 drivers/video/fbdev/controlfb.c        |  6 ++----
 drivers/video/fbdev/efifb.c            |  3 +--
 drivers/video/fbdev/hgafb.c            |  3 +--
 drivers/video/fbdev/hpfb.c             |  3 +--
 drivers/video/fbdev/i810/i810_main.c   |  6 ++----
 drivers/video/fbdev/macfb.c            | 15 +++++----------
 drivers/video/fbdev/offb.c             |  3 +--
 drivers/video/fbdev/pvr2fb.c           |  3 +--
 drivers/video/fbdev/s1d13xxxfb.c       |  3 +--
 drivers/video/fbdev/s3fb.c             |  6 ++----
 drivers/video/fbdev/sh_mobile_hdmi.c   |  6 ++----
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  3 +--
 drivers/video/fbdev/simplefb.c         |  3 +--
 drivers/video/fbdev/stifb.c            |  3 +--
 drivers/video/fbdev/tdfxfb.c           |  6 ++----
 drivers/video/fbdev/tgafb.c            |  3 +--
 drivers/video/fbdev/tridentfb.c        |  6 ++----
 drivers/video/fbdev/valkyriefb.c       |  9 +++------
 drivers/video/fbdev/vesafb.c           |  6 ++----
 drivers/video/fbdev/w100fb.c           |  9 +++------
 22 files changed, 41 insertions(+), 82 deletions(-)
diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index 37ec09b..0f4b147 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -3605,10 +3605,8 @@ err_release_io:
 #ifdef __sparc__
 	kfree(par->mmap_map);
 #else
-	if (par->ati_regbase)
-		iounmap(par->ati_regbase);
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(par->ati_regbase);
+	iounmap(info->screen_base);
 #endif
 err_release_mem:
 	if (par->aux_start)
@@ -3733,13 +3731,10 @@ static void atyfb_remove(struct fb_info *info)
 	}
 #endif
 #ifndef __sparc__
-	if (par->ati_regbase)
-		iounmap(par->ati_regbase);
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(par->ati_regbase);
+	iounmap(info->screen_base);
 #ifdef __BIG_ENDIAN
-	if (info->sprite.addr)
-		iounmap(info->sprite.addr);
+	iounmap(info->sprite.addr);
 #endif
 #endif
 #ifdef __sparc__
diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index d992aa5..d8b3359 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -2164,8 +2164,7 @@ err_release_regions:
 #endif
 	pci_release_regions(pdev);
 err_release_fb:
-	if (cinfo->laguna_mmio != NULL)
-		iounmap(cinfo->laguna_mmio);
+	iounmap(cinfo->laguna_mmio);
 	framebuffer_release(info);
 err_out:
 	return ret;
diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
index 080fdd2..d386fed 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -1068,10 +1068,8 @@ static void control_cleanup(void)
 	if (!p)
 		return;
 
-	if (p->cmap_regs)
-		iounmap(p->cmap_regs);
-	if (p->control_regs)
-		iounmap(p->control_regs);
+	iounmap(p->cmap_regs);
+	iounmap(p->control_regs);
 	if (p->frame_buffer) {
 		if (p->control_use_bank2)
 			p->frame_buffer -= 0x600000;
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..a6d08f0 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -65,8 +65,7 @@ static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
 
 static void efifb_destroy(struct fb_info *info)
 {
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	if (request_mem_succeeded)
 		release_mem_region(info->apertures->ranges[0].base,
 				   info->apertures->ranges[0].size);
diff --git a/drivers/video/fbdev/hgafb.c b/drivers/video/fbdev/hgafb.c
index 5ff9fe2..76b231e 100644
--- a/drivers/video/fbdev/hgafb.c
+++ b/drivers/video/fbdev/hgafb.c
@@ -552,8 +552,7 @@ static int hgafb_probe(struct platform_device *pdev)
 
 	if (! hga_card_detect()) {
 		printk(KERN_INFO "hgafb: HGA card not detected.\n");
-		if (hga_vram)
-			iounmap(hga_vram);
+		iounmap(hga_vram);
 		return -EINVAL;
 	}
 
diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
index a1b7e5f..9b47102 100644
--- a/drivers/video/fbdev/hpfb.c
+++ b/drivers/video/fbdev/hpfb.c
@@ -355,8 +355,7 @@ static void hpfb_remove_one(struct dio_dev *d)
 		iounmap((void *)fb_regs);
 	release_mem_region(d->resource.start, resource_size(&d->resource));
 	fb_dealloc_cmap(&fb_info.cmap);
-	if (fb_info.screen_base)
-		iounmap(fb_info.screen_base);
+	iounmap(fb_info.screen_base);
 }
 
 static struct dio_device_id hpfb_dio_tbl[] = {
diff --git a/drivers/video/fbdev/i810/i810_main.c b/drivers/video/fbdev/i810/i810_main.c
index bb674e4..ce3a585 100644
--- a/drivers/video/fbdev/i810/i810_main.c
+++ b/drivers/video/fbdev/i810/i810_main.c
@@ -2105,10 +2105,8 @@ static void i810fb_release_resource(struct fb_info *info,
 	if (par->i810_gtt.i810_fb_memory)
 		agp_free_memory(gtt->i810_fb_memory);
 
-	if (par->mmio_start_virtual)
-		iounmap(par->mmio_start_virtual);
-	if (par->aperture.virtual)
-		iounmap(par->aperture.virtual);
+	iounmap(par->mmio_start_virtual);
+	iounmap(par->aperture.virtual);
 	kfree(par->edid);
 	if (par->res_flags & FRAMEBUFFER_REQ)
 		release_mem_region(par->aperture.physical,
diff --git a/drivers/video/fbdev/macfb.c b/drivers/video/fbdev/macfb.c
index cda7587..dde0520 100644
--- a/drivers/video/fbdev/macfb.c
+++ b/drivers/video/fbdev/macfb.c
@@ -541,16 +541,11 @@ static void __init macfb_setup(char *options)
 
 static void __init iounmap_macfb(void)
 {
-	if (dafb_cmap_regs)
-		iounmap(dafb_cmap_regs);
-	if (v8_brazil_cmap_regs)
-		iounmap(v8_brazil_cmap_regs);
-	if (rbv_cmap_regs)
-		iounmap(rbv_cmap_regs);
-	if (civic_cmap_regs)
-		iounmap(civic_cmap_regs);
-	if (csc_cmap_regs)
-		iounmap(csc_cmap_regs);
+	iounmap(dafb_cmap_regs);
+	iounmap(v8_brazil_cmap_regs);
+	iounmap(rbv_cmap_regs);
+	iounmap(civic_cmap_regs);
+	iounmap(csc_cmap_regs);
 }
 
 static int __init macfb_init(void)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 43a0a52..5efce99 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -281,8 +281,7 @@ static int offb_set_par(struct fb_info *info)
 
 static void offb_destroy(struct fb_info *info)
 {
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 7c74f58..7aa4ef1 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -841,8 +841,7 @@ static int pvr2fb_common_init(void)
 	return 0;
 
 out_err:
-	if (fb_info->screen_base)
-		iounmap(fb_info->screen_base);
+	iounmap(fb_info->screen_base);
 	if (par->mmio_base)
 		iounmap((void *)par->mmio_base);
 
diff --git a/drivers/video/fbdev/s1d13xxxfb.c b/drivers/video/fbdev/s1d13xxxfb.c
index 83433cb..dff38b7 100644
--- a/drivers/video/fbdev/s1d13xxxfb.c
+++ b/drivers/video/fbdev/s1d13xxxfb.c
@@ -750,8 +750,7 @@ s1d13xxxfb_remove(struct platform_device *pdev)
 
 		fb_dealloc_cmap(&info->cmap);
 
-		if (info->screen_base)
-			iounmap(info->screen_base);
+		iounmap(info->screen_base);
 
 		framebuffer_release(info);
 	}
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index f0ae61a..3d9d45f 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -1382,8 +1382,7 @@ err_find_mode:
 #ifdef CONFIG_FB_S3_DDC
 	if (par->ddc_registered)
 		i2c_del_adapter(&par->ddc_adapter);
-	if (par->mmio)
-		iounmap(par->mmio);
+	iounmap(par->mmio);
 #endif
 	pci_iounmap(dev, info->screen_base);
 err_iomap:
@@ -1419,8 +1418,7 @@ static void s3_pci_remove(struct pci_dev *dev)
 #ifdef CONFIG_FB_S3_DDC
 		if (par->ddc_registered)
 			i2c_del_adapter(&par->ddc_adapter);
-		if (par->mmio)
-			iounmap(par->mmio);
+		iounmap(par->mmio);
 #endif
 
 		pci_iounmap(dev, info->screen_base);
diff --git a/drivers/video/fbdev/sh_mobile_hdmi.c b/drivers/video/fbdev/sh_mobile_hdmi.c
index 7c72a3f..2a7187d 100644
--- a/drivers/video/fbdev/sh_mobile_hdmi.c
+++ b/drivers/video/fbdev/sh_mobile_hdmi.c
@@ -1397,8 +1397,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
 ecodec:
 	free_irq(irq, hdmi);
 ereqirq:
-	if (hdmi->htop1)
-		iounmap(hdmi->htop1);
+	iounmap(hdmi->htop1);
 emap_htop1:
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
@@ -1428,8 +1427,7 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	clk_disable_unprepare(hdmi->hdmi_clk);
 	clk_put(hdmi->hdmi_clk);
-	if (hdmi->htop1)
-		iounmap(hdmi->htop1);
+	iounmap(hdmi->htop1);
 	iounmap(hdmi->base);
 	release_mem_region(res->start, resource_size(res));
 
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index d3013cd..4f9a25b 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -2489,8 +2489,7 @@ static int sh_mobile_lcdc_remove(struct platform_device *pdev)
 		clk_put(priv->dot_clk);
 	}
 
-	if (priv->base)
-		iounmap(priv->base);
+	iounmap(priv->base);
 
 	if (priv->irq)
 		free_irq(priv->irq, priv);
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 92cac80..40e6b33 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -72,8 +72,7 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 static void simplefb_destroy(struct fb_info *info)
 {
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 }
 
 static struct fb_ops simplefb_ops = {
diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 86621fa..ea12305 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -1374,8 +1374,7 @@ stifb_cleanup(void)
 			unregister_framebuffer(sti->info);
 			release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
 		        release_mem_region(info->fix.smem_start, info->fix.smem_len);
-				if (info->screen_base)
-					iounmap(info->screen_base);
+				iounmap(info->screen_base);
 		        fb_dealloc_cmap(&info->cmap);
 		        framebuffer_release(info);
 		}
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index f761fe3..69d4062 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -1572,15 +1572,13 @@ out_err_iobase:
 	release_region(pci_resource_start(pdev, 2),
 		       pci_resource_len(pdev, 2));
 out_err_screenbase:
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	release_mem_region(info->fix.smem_start, pci_resource_len(pdev, 1));
 out_err_regbase:
 	/*
 	 * Cleanup after anything that was remapped/allocated.
 	 */
-	if (default_par->regbase_virt)
-		iounmap(default_par->regbase_virt);
+	iounmap(default_par->regbase_virt);
 	release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
 out_err:
 	framebuffer_release(info);
diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c
index 65ba992..b6fb7e8 100644
--- a/drivers/video/fbdev/tgafb.c
+++ b/drivers/video/fbdev/tgafb.c
@@ -1520,8 +1520,7 @@ static int tgafb_register(struct device *dev)
  err2:
 	fb_dealloc_cmap(&info->cmap);
  err1:
-	if (mem_base)
-		iounmap(mem_base);
+	iounmap(mem_base);
 	release_mem_region(bar0_start, bar0_len);
  err0:
 	framebuffer_release(info);
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7ed9a22..71567401 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -1531,13 +1531,11 @@ static int trident_pci_probe(struct pci_dev *dev,
 
 out_unmap2:
 	kfree(info->pixmap.addr);
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	release_mem_region(tridentfb_fix.smem_start, tridentfb_fix.smem_len);
 	disable_mmio(info->par);
 out_unmap1:
-	if (default_par->io_virt)
-		iounmap(default_par->io_virt);
+	iounmap(default_par->io_virt);
 	release_mem_region(tridentfb_fix.mmio_start, tridentfb_fix.mmio_len);
 	framebuffer_release(info);
 	return err;
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 275fb98..2a9213b 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -400,12 +400,9 @@ int __init valkyriefb_init(void)
  out_cmap_free:
 	fb_dealloc_cmap(&p->info.cmap);
  out_free:
-	if (p->frame_buffer)
-		iounmap(p->frame_buffer);
-	if (p->cmap_regs)
-		iounmap(p->cmap_regs);
-	if (p->valkyrie_regs)
-		iounmap(p->valkyrie_regs);
+	iounmap(p->frame_buffer);
+	iounmap(p->cmap_regs);
+	iounmap(p->valkyrie_regs);
 	kfree(p);
 	return err;
 }
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index d79a0ac..693d18e 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -176,8 +176,7 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
 static void vesafb_destroy(struct fb_info *info)
 {
 	fb_dealloc_cmap(&info->cmap);
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 }
 
@@ -492,8 +491,7 @@ static int vesafb_probe(struct platform_device *dev)
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
 	return 0;
 err:
-	if (info->screen_base)
-		iounmap(info->screen_base);
+	iounmap(info->screen_base);
 	framebuffer_release(info);
 	release_mem_region(vesafb_fix.smem_start, size_total);
 	return err;
diff --git a/drivers/video/fbdev/w100fb.c b/drivers/video/fbdev/w100fb.c
index 10951c8..aeb53eb 100644
--- a/drivers/video/fbdev/w100fb.c
+++ b/drivers/video/fbdev/w100fb.c
@@ -770,12 +770,9 @@ out:
 		fb_dealloc_cmap(&info->cmap);
 		kfree(info->pseudo_palette);
 	}
-	if (remapped_fbuf != NULL)
-		iounmap(remapped_fbuf);
-	if (remapped_regs != NULL)
-		iounmap(remapped_regs);
-	if (remapped_base != NULL)
-		iounmap(remapped_base);
+	iounmap(remapped_fbuf);
+	iounmap(remapped_regs);
+	iounmap(remapped_base);
 	if (info)
 		framebuffer_release(info);
 	return err;
-- 
2.2.2

video: One function call less in tdfxfb_probe() after error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:25:40

From: Markus Elfring <redacted>
Date: Sun, 18 Jan 2015 23:30:41 +0100

The iounmap() function could be called in two cases by the tdfxfb_probe()
function during error handling even if the passed data structure element
contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/tdfxfb.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index 69d4062..c280a3b 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -1438,20 +1438,20 @@ static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!default_par->regbase_virt) {
 		printk(KERN_ERR "fb: Can't remap %s register area.\n",
 				info->fix.id);
-		goto out_err_regbase;
+		goto release_memory_region_regbase;
 	}
 
 	info->fix.smem_start = pci_resource_start(pdev, 1);
 	info->fix.smem_len = do_lfb_size(default_par, pdev->device);
 	if (!info->fix.smem_len) {
 		printk(KERN_ERR "fb: Can't count %s memory.\n", info->fix.id);
-		goto out_err_regbase;
+		goto io_unmap_regbase;
 	}
 
 	if (!request_mem_region(info->fix.smem_start,
 				pci_resource_len(pdev, 1), "tdfx smem")) {
 		printk(KERN_ERR "tdfxfb: Can't reserve smem\n");
-		goto out_err_regbase;
+		goto io_unmap_regbase;
 	}
 
 	info->screen_base = ioremap_nocache(info->fix.smem_start,
@@ -1459,7 +1459,7 @@ static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!info->screen_base) {
 		printk(KERN_ERR "fb: Can't remap %s framebuffer.\n",
 				info->fix.id);
-		goto out_err_screenbase;
+		goto release_memory_region_screenbase;
 	}
 
 	default_par->iobase = pci_resource_start(pdev, 2);
@@ -1467,7 +1467,7 @@ static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!request_region(pci_resource_start(pdev, 2),
 			    pci_resource_len(pdev, 2), "tdfx iobase")) {
 		printk(KERN_ERR "tdfxfb: Can't reserve iobase\n");
-		goto out_err_screenbase;
+		goto io_unmap_screenbase;
 	}
 
 	printk(KERN_INFO "fb: %s memory = %dK\n", info->fix.id,
@@ -1571,14 +1571,16 @@ out_err_iobase:
 			 info->fix.smem_len);
 	release_region(pci_resource_start(pdev, 2),
 		       pci_resource_len(pdev, 2));
-out_err_screenbase:
+io_unmap_screenbase:
 	iounmap(info->screen_base);
+release_memory_region_screenbase:
 	release_mem_region(info->fix.smem_start, pci_resource_len(pdev, 1));
-out_err_regbase:
+io_unmap_regbase:
 	/*
 	 * Cleanup after anything that was remapped/allocated.
 	 */
 	iounmap(default_par->regbase_virt);
+release_memory_region_regbase:
 	release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
 out_err:
 	framebuffer_release(info);
-- 
2.2.2

[PATCH 3/9] video: One function call less in vesafb_probe() after, error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:30:49

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 11:30:33 +0100

The iounmap() function could be called in two cases by the vesafb_probe()
function during error handling even if the passed data structure element
contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/vesafb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 693d18e..de7cad0 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -303,7 +303,7 @@ static int vesafb_probe(struct platform_device *dev)
 	info->apertures = alloc_apertures(1);
 	if (!info->apertures) {
 		err = -ENOMEM;
-		goto err;
+		goto release_framebuffer;
 	}
 	info->apertures->ranges[0].base = screen_info.lfb_base;
 	info->apertures->ranges[0].size = size_total;
@@ -462,7 +462,7 @@ static int vesafb_probe(struct platform_device *dev)
 		       "vesafb: abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
 			vesafb_fix.smem_len, vesafb_fix.smem_start);
 		err = -EIO;
-		goto err;
+		goto release_framebuffer;
 	}
 
 	printk(KERN_INFO "vesafb: framebuffer at 0x%lx, mapped to 0x%p, "
@@ -492,6 +492,7 @@ static int vesafb_probe(struct platform_device *dev)
 	return 0;
 err:
 	iounmap(info->screen_base);
+release_framebuffer:
 	framebuffer_release(info);
 	release_mem_region(vesafb_fix.smem_start, size_total);
 	return err;
-- 
2.2.2

[PATCH 4/9] video: Add check for return value of ioremap() in cirrusfb_pci_register()

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:35:16

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 13:18:50 +0100

The return value from a call of the ioremap() function was not checked
by the implementation of the cirrusfb_pci_register() function.
Let us complete the corresponding error detection and exception handling
at this source code place.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/cirrusfb.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index d8b3359..aa4050c 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -2111,6 +2111,10 @@ static int cirrusfb_pci_register(struct pci_dev *pdev,
 	/* FIXME: this forces VGA.  alternatives? */
 	cinfo->regbase = NULL;
 	cinfo->laguna_mmio = ioremap(info->fix.mmio_start, 0x1000);
+	if (!cinfo->laguna_mmio) {
+		ret = -EIO;
+		goto err_release_fb;
+	}
 
 	dev_dbg(info->device, "Board address: 0x%lx, register address: 0x%lx\n",
 		board_addr, info->fix.mmio_start);
@@ -2122,7 +2126,7 @@ static int cirrusfb_pci_register(struct pci_dev *pdev,
 	if (ret < 0) {
 		dev_err(info->device, "cannot reserve region 0x%lx, abort\n",
 			board_addr);
-		goto err_release_fb;
+		goto io_unmap_laguna;
 	}
 #if 0 /* if the system didn't claim this region, we would... */
 	if (!request_mem_region(0xA0000, 65535, "cirrusfb")) {
@@ -2163,8 +2167,9 @@ err_release_legacy:
 err_release_regions:
 #endif
 	pci_release_regions(pdev);
-err_release_fb:
+io_unmap_laguna:
 	iounmap(cinfo->laguna_mmio);
+err_release_fb:
 	framebuffer_release(info);
 err_out:
 	return ret;
-- 
2.2.2

[PATCH 5/9] video: Less function calls in trident_pci_probe() after error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:38:35

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 14:18:47 +0100

The functions "iounmap" and "kfree" could be called in a few cases by the
trident_pci_probe() function during error handling even if the passed data
structure element contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/tridentfb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 71567401..86d4733 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -1436,7 +1436,7 @@ static int trident_pci_probe(struct pci_dev *dev,
 	if (!default_par->io_virt) {
 		debug("ioremap failed\n");
 		err = -1;
-		goto out_unmap1;
+		goto release_memory_region_mmio;
 	}
 
 	enable_mmio(default_par);
@@ -1459,7 +1459,7 @@ static int trident_pci_probe(struct pci_dev *dev,
 	if (!info->screen_base) {
 		debug("ioremap failed\n");
 		err = -1;
-		goto out_unmap2;
+		goto release_memory_region_smem;
 	}
 
 	default_par->flatpanel = is_flatpanel(default_par);
@@ -1485,7 +1485,7 @@ static int trident_pci_probe(struct pci_dev *dev,
 	info->pixmap.addr = kmalloc(4096, GFP_KERNEL);
 	if (!info->pixmap.addr) {
 		err = -ENOMEM;
-		goto out_unmap2;
+		goto io_unmap_screenbase;
 	}
 
 	info->pixmap.size = 4096;
@@ -1531,11 +1531,14 @@ static int trident_pci_probe(struct pci_dev *dev,
 
 out_unmap2:
 	kfree(info->pixmap.addr);
+io_unmap_screenbase:
 	iounmap(info->screen_base);
+release_memory_region_smem:
 	release_mem_region(tridentfb_fix.smem_start, tridentfb_fix.smem_len);
 	disable_mmio(info->par);
 out_unmap1:
 	iounmap(default_par->io_virt);
+release_memory_region_mmio:
 	release_mem_region(tridentfb_fix.mmio_start, tridentfb_fix.mmio_len);
 	framebuffer_release(info);
 	return err;
-- 
2.2.2

[PATCH 6/9] video: One function call less in pvr2fb_common_init() after error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:41:07

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 14:50:44 +0100

The iounmap() function could be called in two cases by the pvr2fb_common_init()
function during error handling even if the passed data structure element
contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/pvr2fb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 7aa4ef1..9fb8bbf 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -773,14 +773,14 @@ static int pvr2fb_common_init(void)
 
 	if (!fb_info->screen_base) {
 		printk(KERN_ERR "pvr2fb: Failed to remap smem space\n");
-		goto out_err;
+		return -ENXIO;
 	}
 
 	par->mmio_base = (unsigned long)ioremap_nocache(pvr2_fix.mmio_start,
 							pvr2_fix.mmio_len);
 	if (!par->mmio_base) {
 		printk(KERN_ERR "pvr2fb: Failed to remap mmio space\n");
-		goto out_err;
+		goto io_unmap_screen;
 	}
 
 	fb_memset(fb_info->screen_base, 0, pvr2_fix.smem_len);
@@ -807,7 +807,7 @@ static int pvr2fb_common_init(void)
 	fb_alloc_cmap(&fb_info->cmap, 256, 0);
 
 	if (register_framebuffer(fb_info) < 0)
-		goto out_err;
+		goto io_unmap_mmio;
 	/*Must write PIXDEPTH to register before anything is displayed - so force init */
 	pvr2_init_display(fb_info);
 
@@ -840,10 +840,10 @@ static int pvr2fb_common_init(void)
 
 	return 0;
 
-out_err:
+io_unmap_mmio:
+	iounmap((void *)par->mmio_base);
+io_unmap_screen:
 	iounmap(fb_info->screen_base);
-	if (par->mmio_base)
-		iounmap((void *)par->mmio_base);
 
 	return -ENXIO;
 }
-- 
2.2.2

[PATCH 7/9] video: One function call less in tgafb_register() after, error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:43:48

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 15:30:45 +0100

The iounmap() function could be called in one case by the tgafb_register()
function during error handling even if the passed variable "mem_base"
contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels
(according also to current Linux coding style).

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/tgafb.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c
index b6fb7e8..a3cd0ef 100644
--- a/drivers/video/fbdev/tgafb.c
+++ b/drivers/video/fbdev/tgafb.c
@@ -1436,14 +1436,14 @@ static int tgafb_register(struct device *dev)
 	}
 	if (!request_mem_region (bar0_start, bar0_len, "tgafb")) {
 		printk(KERN_ERR "tgafb: cannot reserve FB region\n");
-		goto err0;
+		goto release_framebuffer;
 	}
 
 	/* Map the framebuffer.  */
 	mem_base = ioremap_nocache(bar0_start, bar0_len);
 	if (!mem_base) {
 		printk(KERN_ERR "tgafb: Cannot map MMIO\n");
-		goto err1;
+		goto release_memory_region;
 	}
 
 	/* Grab info about the card.  */
@@ -1484,13 +1484,13 @@ static int tgafb_register(struct device *dev)
 	if (ret = 0 || ret = 4) {
 		printk(KERN_ERR "tgafb: Could not find valid video mode\n");
 		ret = -EINVAL;
-		goto err1;
+		goto io_unmap;
 	}
 
 	if (fb_alloc_cmap(&info->cmap, 256, 0)) {
 		printk(KERN_ERR "tgafb: Could not allocate color map\n");
 		ret = -ENOMEM;
-		goto err1;
+		goto io_unmap;
 	}
 
 	tgafb_set_par(info);
@@ -1498,7 +1498,7 @@ static int tgafb_register(struct device *dev)
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "tgafb: Could not register framebuffer\n");
 		ret = -EINVAL;
-		goto err2;
+		goto dealloc_cmap;
 	}
 
 	if (tga_bus_pci) {
@@ -1517,12 +1517,13 @@ static int tgafb_register(struct device *dev)
 
 	return 0;
 
- err2:
+dealloc_cmap:
 	fb_dealloc_cmap(&info->cmap);
- err1:
+io_unmap:
 	iounmap(mem_base);
+release_memory_region:
 	release_mem_region(bar0_start, bar0_len);
- err0:
+release_framebuffer:
 	framebuffer_release(info);
 	return ret;
 }
-- 
2.2.2

[PATCH 8/9] video: Less function calls in valkyriefb_init() after error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:47:04

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 17:01:25 +0100

The iounmap() function could be called in three cases by the valkyriefb_init()
function during error handling even if the passed data structure element
contained still a null pointer.

This implementation detail could be improved by adjustments for jump labels
(according also to current Linux coding style) and error messages.

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/valkyriefb.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 2a9213b..a81a0f1 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -369,40 +369,50 @@ int __init valkyriefb_init(void)
 	}
 	p->total_vram = 0x100000;
 	p->frame_buffer_phys = frame_buffer_phys;
+	err = -ENOMEM;
 	p->frame_buffer = __ioremap(frame_buffer_phys, p->total_vram, flags);
+	if (p->frame_buffer = NULL) {
+		pr_err("%s: %s failed\n", __func__, "Frame buffer mapping");
+		goto free_memory;
+	}
 	p->cmap_regs_phys = cmap_regs_phys;
 	p->cmap_regs = ioremap(p->cmap_regs_phys, 0x1000);
+	if (p->cmap_regs = NULL) {
+		pr_err("%s: %s failed\n", __func__, "cmap");
+		goto io_unmap_framebuffer;
+	}
 	p->valkyrie_regs_phys = cmap_regs_phys+0x6000;
 	p->valkyrie_regs = ioremap(p->valkyrie_regs_phys, 0x1000);
-	err = -ENOMEM;
-	if (p->frame_buffer = NULL || p->cmap_regs = NULL
-	    || p->valkyrie_regs = NULL) {
-		printk(KERN_ERR "valkyriefb: couldn't map resources\n");
-		goto out_free;
+	if (p->valkyrie_regs = NULL) {
+		pr_err("%s: %s failed\n", __func__, "ioremap");
+		goto io_unmap_cmap;
 	}
 
 	valkyrie_choose_mode(p);
 	mac_vmode_to_var(default_vmode, default_cmode, &p->info.var);
 	err = valkyrie_init_info(&p->info, p);
 	if (err < 0)
-		goto out_free;
+		goto io_unmap_valkyrie;
 	valkyrie_init_fix(&p->info.fix, p);
 	if (valkyriefb_set_par(&p->info))
 		/* "can't happen" */
 		printk(KERN_ERR "valkyriefb: can't set default video mode\n");
 
 	if ((err = register_framebuffer(&p->info)) != 0)
-		goto out_cmap_free;
+		goto dealloc_cmap;
 
 	fb_info(&p->info, "valkyrie frame buffer device\n");
 	return 0;
 
- out_cmap_free:
+dealloc_cmap:
 	fb_dealloc_cmap(&p->info.cmap);
- out_free:
-	iounmap(p->frame_buffer);
-	iounmap(p->cmap_regs);
+io_unmap_valkyrie:
 	iounmap(p->valkyrie_regs);
+io_unmap_cmap:
+	iounmap(p->cmap_regs);
+io_unmap_framebuffer:
+	iounmap(p->frame_buffer);
+free_memory:
 	kfree(p);
 	return err;
 }
-- 
2.2.2

[PATCH 9/9] video: Less function calls in w100fb_probe() after error detection

From: SF Markus Elfring <hidden>
Date: 2015-01-19 18:49:53

From: Markus Elfring <redacted>
Date: Mon, 19 Jan 2015 17:56:11 +0100

The iounmap() function could be called in three cases by the w100fb_probe()
function during error handling even if the passed data structure element
contained still a null pointer.

* This implementation detail could be improved by adjustments for
  jump labels.

* Let us also delete two unnecessary null pointer checks for the
  variable "info".

Signed-off-by: Markus Elfring <redacted>
---
 drivers/video/fbdev/w100fb.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/w100fb.c b/drivers/video/fbdev/w100fb.c
index aeb53eb..67ddc57 100644
--- a/drivers/video/fbdev/w100fb.c
+++ b/drivers/video/fbdev/w100fb.c
@@ -645,12 +645,12 @@ int w100fb_probe(struct platform_device *pdev)
 	/* Remap the chip base address */
 	remapped_base = ioremap_nocache(mem->start+W100_CFG_BASE, W100_CFG_LEN);
 	if (remapped_base = NULL)
-		goto out;
+		goto exit;
 
 	/* Map the register space */
 	remapped_regs = ioremap_nocache(mem->start+W100_REG_BASE, W100_REG_LEN);
 	if (remapped_regs = NULL)
-		goto out;
+		goto io_unmap_base;
 
 	/* Identify the chip */
 	printk("Found ");
@@ -662,19 +662,19 @@ int w100fb_probe(struct platform_device *pdev)
 		default:
 			printk("Unknown imageon chip ID\n");
 			err = -ENODEV;
-			goto out;
+			goto io_unmap_regs;
 	}
 	printk(" at 0x%08lx.\n", (unsigned long) mem->start+W100_CFG_BASE);
 
 	/* Remap the framebuffer */
 	remapped_fbuf = ioremap_nocache(mem->start+MEM_WINDOW_BASE, MEM_WINDOW_SIZE);
 	if (remapped_fbuf = NULL)
-		goto out;
+		goto io_unmap_regs;
 
 	info=framebuffer_alloc(sizeof(struct w100fb_par), &pdev->dev);
 	if (!info) {
 		err = -ENOMEM;
-		goto out;
+		goto io_unmap_framebuffer;
 	}
 
 	par = info->par;
@@ -690,13 +690,13 @@ int w100fb_probe(struct platform_device *pdev)
 	if (!par->pll_table) {
 		printk(KERN_ERR "No matching Xtal definition found\n");
 		err = -EINVAL;
-		goto out;
+		goto release_framebuffer;
 	}
 
 	info->pseudo_palette = kmalloc(sizeof (u32) * MAX_PALETTES, GFP_KERNEL);
 	if (!info->pseudo_palette) {
 		err = -ENOMEM;
-		goto out;
+		goto release_framebuffer;
 	}
 
 	info->fbops = &w100fb_ops;
@@ -716,7 +716,7 @@ int w100fb_probe(struct platform_device *pdev)
 
 	if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) {
 		err = -ENOMEM;
-		goto out;
+		goto free_palette;
 	}
 
 	par->mode = &inf->modelist[0];
@@ -766,15 +766,18 @@ int w100fb_probe(struct platform_device *pdev)
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
 	return 0;
 out:
-	if (info) {
-		fb_dealloc_cmap(&info->cmap);
-		kfree(info->pseudo_palette);
-	}
+	fb_dealloc_cmap(&info->cmap);
+free_palette:
+	kfree(info->pseudo_palette);
+release_framebuffer:
+	framebuffer_release(info);
+io_unmap_framebuffer:
 	iounmap(remapped_fbuf);
+io_unmap_regs:
 	iounmap(remapped_regs);
+io_unmap_base:
 	iounmap(remapped_base);
-	if (info)
-		framebuffer_release(info);
+exit:
 	return err;
 }
 
-- 
2.2.2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help