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

Subsystems: framebuffer layer, the rest

STALE5227d

9 messages, 6 authors, 2012-04-25 · open the first message on its own page

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

From: Julia Lawall <hidden>
Date: 2012-04-18 19:37:18

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>

---
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);

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

From: H Hartley Sweeten <hidden>
Date: 2012-04-18 20:35:57

On Wednesday, April 18, 2012 12:37 PM, Julia Lawall wrote:

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>

Thanks!

Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>

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

From: Ryan Mallon <hidden>
Date: 2012-04-19 00:16:00

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

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?

~Ryan

quoted hunk

---
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/

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

From: Jingoo Han <hidden>
Date: 2012-04-19 02:16:10

-----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

~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

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

From: Julia Lawall <hidden>
Date: 2012-04-19 05:14:11

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

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

From: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Date: 2012-04-23 06:19:19

On 04/18/2012 07:37 PM, Julia Lawall wrote:

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>

Applied.


Thanks,

Florian Tobias Schandinat

quoted hunk

---
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);

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

From: Joe Perches <joe@perches.com>
Date: 2012-04-24 18:10:40

On Thu, 2012-04-19 at 07:14 +0200, Julia Lawall wrote:

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

[]

quoted

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)

Prefixing "./" to the file path bypasses the normal
get_maintainers pattern check for file maintainership.

Florian Schandinat and linux-fbdev are not returned
in the second lookup above.

One of the patterns for FRAMEBUFFER is
F:	drivers/video/
which is a direct match for is ep93xx-fb.c file,
so --git is not used by default.

Adding --git may be appropriate and it also returns
Ryan's name in this case.

$ ./scripts/get_maintainer.pl -f 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)

$ ./scripts/get_maintainer.pl --git -f drivers/video/ep93xx-fb.c
Florian Tobias Schandinat [off-list ref] (maintainer:FRAMEBUFFER LAYER)
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-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)

Nominally, as framebuffer maintainer, Florian
should also have "signed-off" on these commits,
but many subsystems don't actually have a
single path or person for patches and commits.

This file came into the tree via Andrew Morton.

Another way to investigate who actually does
work on an individual file is to use the
get_maintainer.pl option "--interactive".

That can give you an output like:

*  # email/list and role:stats                                        auth sign
*  1 Florian Tobias Schandinat [off-list ref]             
     maintainer:FRAMEBUFFER LAYER
*  2 Ryan Mallon [off-list ref]                                     4    4
     commit_signer:2/3g%,authored lines:645/650™%,commits:4/8P%
*  3 Paul Gortmaker [off-list ref]                       1    1
     commit_signer:1/33%,commits:1/8%
*  4 H Hartley Sweeten [off-list ref]                    0    2
     commit_signer:1/33%,commits:2/8%%
*  5 Jesper Juhl [off-list ref]                                      0    1
     commit_signer:1/33%
*  6 Jiri Kosina [off-list ref]                                       0    1
     commit_signer:1/33%
*  7 Andrew Morton [off-list ref]                           0    2
     commits:2/8%%
*  8 Russell King [off-list ref]                          0    1
     commits:1/8%
*  9 linux-fbdev@vger.kernel.org                                      
     open list:FRAMEBUFFER LAYER
* 10 linux-kernel@vger.kernel.org                                     
     open list

About maintainership for individual files:

If anyone thinks they are the maintainer for
a particular file, generally they should submit
a patch to MAINTAINERS adding a section with
their name, maintainership level and an
appropriate file pattern.

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

From: Ryan Mallon <hidden>
Date: 2012-04-24 21:10:13

On 24/04/12 22:46, Joe Perches wrote:

On Thu, 2012-04-19 at 07:14 +0200, Julia Lawall wrote:

quoted

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

[]

quoted

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)

Prefixing "./" to the file path bypasses the normal
get_maintainers pattern check for file maintainership.

Florian Schandinat and linux-fbdev are not returned
in the second lookup above.

One of the patterns for FRAMEBUFFER is
F:	drivers/video/
which is a direct match for is ep93xx-fb.c file,
so --git is not used by default.

Adding --git may be appropriate and it also returns
Ryan's name in this case.


Hi Joe,

Thanks for the info.

Nominally, as framebuffer maintainer, Florian
should also have "signed-off" on these commits,
but many subsystems don't actually have a
single path or person for patches and commits.

This file came into the tree via Andrew Morton.

The original driver was written before Florian became the 
framebuffer maintainer. The subsystem had no real maintainer
at the time, which is why it went via Andrew.

About maintainership for individual files:

If anyone thinks they are the maintainer for
a particular file, generally they should submit
a patch to MAINTAINERS adding a section with
their name, maintainership level and an
appropriate file pattern.


Hmm, perhaps author is a better term than maintainer here. Any 
patches should still go via the framebuffer maintainer, Florian,
but it would be useful in many cases for the original and/or
most regular commit author to also get Cc'ed since in many
cases they will have a better understanding of the code, even
if the aren't "maintaining" it.

~Ryan

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

From: Jingoo Han <hidden>
Date: 2012-04-25 01:19:19

On Tue, 24 Apr 2012, Joe Perches wrote:

-----Original Message-----
On Thu, 2012-04-19 at 07:14 +0200, Julia Lawall wrote:

quoted

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

[]

quoted

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)

Prefixing "./" to the file path bypasses the normal
get_maintainers pattern check for file maintainership.

Florian Schandinat and linux-fbdev are not returned
in the second lookup above.

One of the patterns for FRAMEBUFFER is
F:	drivers/video/
which is a direct match for is ep93xx-fb.c file,
so --git is not used by default.

Adding --git may be appropriate and it also returns
Ryan's name in this case.

$ ./scripts/get_maintainer.pl -f 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)

$ ./scripts/get_maintainer.pl --git -f drivers/video/ep93xx-fb.c
Florian Tobias Schandinat [off-list ref] (maintainer:FRAMEBUFFER LAYER)
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-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)

Nominally, as framebuffer maintainer, Florian
should also have "signed-off" on these commits,
but many subsystems don't actually have a
single path or person for patches and commits.

This file came into the tree via Andrew Morton.

Another way to investigate who actually does
work on an individual file is to use the
get_maintainer.pl option "--interactive".

Thank you for your reply.
It is very helpful.

Best regards,
Jingoo Han

That can give you an output like:

*  # email/list and role:stats                                        auth sign
*  1 Florian Tobias Schandinat [off-list ref]
     maintainer:FRAMEBUFFER LAYER
*  2 Ryan Mallon [off-list ref]                                     4    4
     commit_signer:2/3g%,authored lines:645/650™%,commits:4/8P%
*  3 Paul Gortmaker [off-list ref]                       1    1
     commit_signer:1/33%,commits:1/8%
*  4 H Hartley Sweeten [off-list ref]                    0    2
     commit_signer:1/33%,commits:2/8%%
*  5 Jesper Juhl [off-list ref]                                      0    1
     commit_signer:1/33%
*  6 Jiri Kosina [off-list ref]                                       0    1
     commit_signer:1/33%
*  7 Andrew Morton [off-list ref]                           0    2
     commits:2/8%%
*  8 Russell King [off-list ref]                          0    1
     commits:1/8%
*  9 linux-fbdev@vger.kernel.org
     open list:FRAMEBUFFER LAYER
* 10 linux-kernel@vger.kernel.org
     open list

About maintainership for individual files:

If anyone thinks they are the maintainer for
a particular file, generally they should submit
a patch to MAINTAINERS adding a section with
their name, maintainership level and an
appropriate file pattern.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help