checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Jonathan Cameron]

This started happening when I run checkpatch on pretty much anything.

triggered by the b in this snipped for example


diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
index 3f8d0af..62dc2a8 100644
--- a/arch/arm/mach-pxa/stargate2.c
+++ b/arch/arm/mach-pxa/stargate2.c
@@ -54,6 +54,8 @@
 #include <linux/mfd/da903x.h>


Anyone else seeing this or have a clue what the heck is going on?

Jonathan

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[David Rientjes]

On Tue, 18 Oct 2011, Jonathan Cameron wrote:

> This started happening when I run checkpatch on pretty much anything.
> 
> triggered by the b in this snipped for example
> 
> 
> diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
> index 3f8d0af..62dc2a8 100644
> --- a/arch/arm/mach-pxa/stargate2.c
> +++ b/arch/arm/mach-pxa/stargate2.c
> @@ -54,6 +54,8 @@
>  #include <linux/mfd/da903x.h>
> 
> 
> Anyone else seeing this or have a clue what the heck is going on?
> 

I don't think anyone can say for sure if they're seeing what you're seeing 
since checkpatch will just complain that this is a corrupt patch.  Could 
you send an actual diff that exhibits the problem?

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Joe Perches]

On Tue, 2011-10-18 at 16:44 -0700, David Rientjes wrote:
> On Tue, 18 Oct 2011, Jonathan Cameron wrote:
> > This started happening when I run checkpatch on pretty much anything.
> > triggered by the b in this snipped for example
> > diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
> > index 3f8d0af..62dc2a8 100644
> > --- a/arch/arm/mach-pxa/stargate2.c
> > +++ b/arch/arm/mach-pxa/stargate2.c
> > @@ -54,6 +54,8 @@
> >  #include <linux/mfd/da903x.h>
> > Anyone else seeing this or have a clue what the heck is going on?

It's done this since:

$ git log -1 1e85572697b348b1a126520349a29654f2ae6a12
commit 1e85572697b348b1a126520349a29654f2ae6a12
Author: Wolfram Sang <w.sang@pengutronix.de>
Date:   Tue Jan 6 14:41:24 2009 -0800

    checkpatch: Add warning for p0-patches
    
    Some people work internally with -p0-patches which has the danger that one
    forgets to convert them to -p1 before mainlining.  Bitten myself and seen
    p0-patches in mailing lists occasionally, this patch adds a warning to
    checkpatch.pl in case a patch is -p0.  If you really want, you can fool
    this check to generate false positives, this is why it just spits a
    warning.  Making the check 100% proof is trickier than it looks, so let's
    start with a version which catches the cases of real use.
    
    [apw@canonical.com: update message language, handle null prefix, add tests]
    Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
    Signed-off-by: Andy Whitcroft <apw@canonical.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

I've always ignored it.

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[David Rientjes]

On Tue, 18 Oct 2011, Joe Perches wrote:

> > > This started happening when I run checkpatch on pretty much anything.
> > > triggered by the b in this snipped for example
> > > diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
> > > index 3f8d0af..62dc2a8 100644
> > > --- a/arch/arm/mach-pxa/stargate2.c
> > > +++ b/arch/arm/mach-pxa/stargate2.c
> > > @@ -54,6 +54,8 @@
> > >  #include <linux/mfd/da903x.h>
> > > Anyone else seeing this or have a clue what the heck is going on?
> 
> It's done this since:
> 
> $ git log -1 1e85572697b348b1a126520349a29654f2ae6a12
> commit 1e85572697b348b1a126520349a29654f2ae6a12
> Author: Wolfram Sang <w.sang@pengutronix.de>
> Date:   Tue Jan 6 14:41:24 2009 -0800
> 
>     checkpatch: Add warning for p0-patches
>     

Hmm, not sure how useful that is unless both prefixes ('a' and 'b') exist.

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On 10/19/11 02:27, David Rientjes wrote:
> On Tue, 18 Oct 2011, Joe Perches wrote:
> 
>>>> This started happening when I run checkpatch on pretty much anything.
>>>> triggered by the b in this snipped for example
>>>> diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
>>>> index 3f8d0af..62dc2a8 100644
>>>> --- a/arch/arm/mach-pxa/stargate2.c
>>>> +++ b/arch/arm/mach-pxa/stargate2.c
>>>> @@ -54,6 +54,8 @@
>>>>  #include <linux/mfd/da903x.h>
>>>> Anyone else seeing this or have a clue what the heck is going on?
>>
>> It's done this since:
>>
>> $ git log -1 1e85572697b348b1a126520349a29654f2ae6a12
>> commit 1e85572697b348b1a126520349a29654f2ae6a12
>> Author: Wolfram Sang <w.sang@pengutronix.de>
>> Date:   Tue Jan 6 14:41:24 2009 -0800
>>
>>     checkpatch: Add warning for p0-patches
>>     
> 
> Hmm, not sure how useful that is unless both prefixes ('a' and 'b') exist.
Sorry, I missed one detail. This warning is firing on every patch including
any I pull off mailing lists or produce with git format-patch

As you can imagine - this is rather tedious.

As a random example see the attached patch from yesterday.

Jonathan

[-- Attachment #2: [PATCHv3 6_7] input_cma3000_d0x: Unwind reverse order of init.eml --]
[-- Type: message/rfc822, Size: 3552 bytes --]

From: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
To: dmitry.torokhov@gmail.com, sameo@linux.intel.com, peter.ujfalusi@ti.com, aghayal@codeaurora.org, david@hardeman.nu, Shubhrajyoti@ti.com, saaguirre@ti.com, jic23@cam.ac.uk, hemanthv@ti.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Subject: [PATCHv3 6/7] input/cma3000_d0x: Unwind reverse order of init
Date: Tue, 18 Oct 2011 17:48:05 +0200
Message-ID: <1318952886-835-7-git-send-email-ricardo.ribalda@gmail.com>


Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/input/misc/cma3000_d0x.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/cma3000_d0x.c b/drivers/input/misc/cma3000_d0x.c
index bbda34c..96a46d4 100644
--- a/drivers/input/misc/cma3000_d0x.c
+++ b/drivers/input/misc/cma3000_d0x.c
@@ -459,8 +459,8 @@ EXPORT_SYMBOL(cma3000_init);
 
 void cma3000_exit(struct cma3000_accl_data *data)
 {
-	free_irq(data->irq, data);
 	input_unregister_device(data->input_dev);
+	free_irq(data->irq, data);
 	kfree(data);
 }
 EXPORT_SYMBOL(cma3000_exit);
-- 
1.7.7

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]


> Sorry, I missed one detail. This warning is firing on every patch including
> any I pull off mailing lists or produce with git format-patch

So, I assume you have a directory named 'b' in the kernel-root-dir?

The p1-detection is heuristic and can always be fooled, so I don't think
extending the logic will help. If too many users think it is annoying,
it may be better to remove it. I do think, however, that it might help
users sending in their first patches, so I'd prefer to keep it.
Experienced users have the option to use something like "--ignore
PATCH_PREFIX" in their .checkpatch.conf. I vote for improving the error
message but won't oppose a removal if that is what is wanted.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On 10/19/11 11:26, Wolfram Sang wrote:
> 
>> Sorry, I missed one detail. This warning is firing on every patch including
>> any I pull off mailing lists or produce with git format-patch
> 
> So, I assume you have a directory named 'b' in the kernel-root-dir?
File, but good call.  Thanks.  All back to normal now.
I'll have to be more creative with naming my random temporary diff files ;)
(or remember to delete them afterwards).
> 
> The p1-detection is heuristic and can always be fooled, so I don't think
> extending the logic will help. If too many users think it is annoying,
> it may be better to remove it. I do think, however, that it might help
> users sending in their first patches, so I'd prefer to keep it.
> Experienced users have the option to use something like "--ignore
> PATCH_PREFIX" in their .checkpatch.conf. I vote for improving the error
> message but won't oppose a removal if that is what is wanted.

Maybe something as simple as a comment in the checkpatch source
to say that such a file / directory can cause false positives?

Thanks again

Jonathan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

> Maybe something as simple as a comment in the checkpatch source
> to say that such a file / directory can cause false positives?

I'd hope this error message would be clear enough, what do you think?

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3dfc471..19e4de7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1506,7 +1506,7 @@ sub process {
 			if (!$file && $tree && $p1_prefix ne '' &&
 			    -e "$root/$p1_prefix") {
 				WARN("PATCH_PREFIX",
-				     "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
+				     "patch prefix '$p1_prefix' exists as file/directory. Make sure this isn't a -p0 patch\n");
 			}
 
 			if ($realfile =~ m@^include/asm/@) {

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

On 10/19/11 12:22, Wolfram Sang wrote:
>> Maybe something as simple as a comment in the checkpatch source
>> to say that such a file / directory can cause false positives?
> 
> I'd hope this error message would be clear enough, what do you think?
Would have told me what was going on!

Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3dfc471..19e4de7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1506,7 +1506,7 @@ sub process {
>  			if (!$file && $tree && $p1_prefix ne '' &&
>  			    -e "$root/$p1_prefix") {
>  				WARN("PATCH_PREFIX",
> -				     "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
> +				     "patch prefix '$p1_prefix' exists as file/directory. Make sure this isn't a -p0 patch\n");
>  			}
>  
>  			if ($realfile =~ m@^include/asm/@) {
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

Re: checkpatch WARNING: patch prefix 'b' exists, appears to be a -p0 patch

[Joe Perches]

On Wed, 2011-10-19 at 13:22 +0200, Wolfram Sang wrote:
> > Maybe something as simple as a comment in the checkpatch source
> > to say that such a file / directory can cause false positives?
> 
> I'd hope this error message would be clear enough, what do you think?
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3dfc471..19e4de7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1506,7 +1506,7 @@ sub process {
>  			if (!$file && $tree && $p1_prefix ne '' &&
>  			    -e "$root/$p1_prefix") {
>  				WARN("PATCH_PREFIX",
> -				     "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
> +				     "patch prefix '$p1_prefix' exists as file/directory. Make sure this isn't a -p0 patch\n");
>  			}
>  
>  			if ($realfile =~ m@^include/asm/@) {
> 

How about:

 scripts/checkpatch.pl |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2300964..6259b12 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1499,17 +1499,17 @@ sub process {
 			$realfile =~ s@^([^/]*)/@@;
 
 		} elsif ($line =~ /^\+\+\+\s+(\S+)/) {
+			my $patched_file = "$root/$1";
 			$realfile = $1;
 			$realfile =~ s@^([^/]*)/@@;
 
-			$p1_prefix = $1;
-			if (!$file && $tree && $p1_prefix ne '' &&
-			    -e "$root/$p1_prefix") {
+			if (!$file && $tree && $realfile ne '' &&
+			    -e "$patched_file") {
 				WARN("PATCH_PREFIX",
-				     "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
+				     "patched file '$patched_file' exists, patch appears to be a -p0 patch\n");
 			}
 
-			if ($realfile =~ m@^include/asm/@) {
+			if ($patched_file =~ m@/include/asm/@) {
 				ERROR("MODIFIED_INCLUDE_ASM",
 				      "do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n");
 			}