RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code

I don't know thw impact of the ./.  I use the options

--nokeywords --nogit-fallback --subsystem --norolestats -f

julia

On Thu, 19 Apr 2012, Jingoo Han wrote:

quoted

-----Original Message-----
From: Ryan Mallon
Sent: Thursday, April 19, 2012 9:16 AM
To: Julia Lawall
Cc: Florian Tobias Schandinat; kernel-janitors@vger.kernel.org; linux-fbdev@vger.kernel.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code

On 19/04/12 05:37, Julia Lawall wrote:

quoted

From: Julia Lawall <redacted>

There were two problems in this code: failure of the setup function should
free locally allocated resources like other nearby failures, and the test
if (&info->cmap) can never be false.  To generally clean things up, this
patch reorders the error handling code at the failed label and adds labels
so that the conditionals are not necessary.

Signed-off-by: Julia Lawall <redacted>

Reviewed-by: Ryan Mallon <redacted>

Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
though, according to git blame, I am the author of 90% of the commits.
Should I have an entry in the MAINTAINERS file, or is
scripts/get_maintainer.pl not working properly?

There are optional differences in using scripts/get_maintainer.pl.
If you use './' ahead of file path, you will see your name.

Without './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file drivers/video/ep93xx-fb.c
Florian Tobias Schandinat [off-list ref] (maintainer:FRAMEBUFFER LAYER)
linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)

With './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file ./drivers/video/ep93xx-fb.c
Ryan Mallon [off-list ref] (commit_signer:2/3g%)
Paul Gortmaker [off-list ref] (commit_signer:1/33%)
H Hartley Sweeten [off-list ref] (commit_signer:1/33%)
Jesper Juhl [off-list ref] (commit_signer:1/33%)
Jiri Kosina [off-list ref] (commit_signer:1/33%)
linux-kernel@vger.kernel.org (open list)


Best regards,
Jingoo Han

quoted

~Ryan

quoted

---
Not tested.

 drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
index f8babbe..345d962 100644
--- a/drivers/video/ep93xx-fb.c
+++ b/drivers/video/ep93xx-fb.c

@@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)

 	err = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (err)
-		goto failed;
+		goto failed_cmap;

 	err = ep93xxfb_alloc_videomem(info);
 	if (err)
-		goto failed;
+		goto failed_videomem;

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		err = -ENXIO;
-		goto failed;
+		goto failed_resource;
 	}

 	/*

@@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	fbi->mmio_base = ioremap(res->start, resource_size(res));
 	if (!fbi->mmio_base) {
 		err = -ENXIO;
-		goto failed;
+		goto failed_resource;
 	}

 	strcpy(info->fix.id, pdev->name);

@@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	if (err = 0) {
 		dev_err(info->dev, "No suitable video mode found\n");
 		err = -EINVAL;
-		goto failed;
+		goto failed_mode;
 	}

 	if (mach_info->setup) {
 		err = mach_info->setup(pdev);
 		if (err)
-			return err;
+			goto failed_mode;
 	}

 	err = ep93xxfb_check_var(&info->var, info);
 	if (err)
-		goto failed;
+		goto failed_check;

 	fbi->clk = clk_get(info->dev, NULL);
 	if (IS_ERR(fbi->clk)) {
 		err = PTR_ERR(fbi->clk);
 		fbi->clk = NULL;
-		goto failed;
+		goto failed_check;
 	}

 	ep93xxfb_set_par(info);

@@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	return 0;

 failed:
-	if (fbi->clk)
-		clk_put(fbi->clk);
-	if (fbi->mmio_base)
-		iounmap(fbi->mmio_base);
-	ep93xxfb_dealloc_videomem(info);
-	if (&info->cmap)
-		fb_dealloc_cmap(&info->cmap);
+	clk_put(fbi->clk);
+failed_check:
 	if (fbi->mach_info->teardown)
 		fbi->mach_info->teardown(pdev);
+failed_mode:
+	iounmap(fbi->mmio_base);
+failed_resource:
+	ep93xxfb_dealloc_videomem(info);
+failed_videomem:
+	fb_dealloc_cmap(&info->cmap);
+failed_cmap:
 	kfree(info);
 	platform_set_drvdata(pdev, NULL);


--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html