[PATCH] relax check regex for revert commit

[PATCH] relax check regex for revert commit

[Jia He]

For revert commit, it might has two double quotation marks in its
commit log.
Relax the check condition for revert commit to avoid checkpatch
errors.

Without this patch, checkpatch.pl will report errors:
ERROR: Please use git commit description style
...

Attached testcases here:
[test case 1] test single line revert commit
============================================
>From 9639c10d1c2ae17ca83a1d446b146a23cbaf9a92 Mon Sep 17 00:00:00 2001
From: Jia He <hejianet@gmail.com>
Date: Wed, 28 Mar 2018 23:32:20 -0700
Subject: [PATCH v5 1/5] mm: page_alloc: remain memblock_next_valid_pfn() on

Commit f5c1350256fb ("Revert "mm/page_alloc: fix memmap_init_zone pageblock alignment"")

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
---
 arch/arm/include/asm/page.h   |  2 ++

diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index 4355f0e..489875c 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -158,6 +158,8 @@ typedef struct page *pgtable_t;
 
 #ifdef CONFIG_HAVE_ARCH_PFN_VALID
 extern int pfn_valid(unsigned long);
+extern unsigned long memblock_next_valid_pfn(unsigned long pfn);
+#define skip_to_last_invalid_pfn(pfn) (memblock_next_valid_pfn(pfn) - 1)
 #endif

[test case 2] test multiline revert commt
=========================================
>From 9639c10d1c2ae17ca83a1d446b146a23cbaf9a92 Mon Sep 17 00:00:00 2001
From: Jia He <hejianet@gmail.com>
Date: Wed, 28 Mar 2018 23:32:20 -0700
Subject: [PATCH v5 1/5] mm: page_alloc: remain memblock_next_valid_pfn() on

Commit f5c1350256fb ("Revert "mm/page_alloc: fix memmap_init_zone pageblock
alignment"")

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
---
 arch/arm/include/asm/page.h   |  2 ++

diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index 4355f0e..489875c 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -158,6 +158,8 @@ typedef struct page *pgtable_t;
 
 #ifdef CONFIG_HAVE_ARCH_PFN_VALID
 extern int pfn_valid(unsigned long);
+extern unsigned long memblock_next_valid_pfn(unsigned long pfn);
+#define skip_to_last_invalid_pfn(pfn) (memblock_next_valid_pfn(pfn) - 1)
 #endif

Jia He (1):
  checkpatch: relax check for revert commit

 scripts/checkpatch.pl | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH] checkpatch: relax check for revert commit

[Jia He]

For revert commit, it might has two double quotation marks in its
commit log.

Relax the check condition for revert commit to avoid checkpatch
errors.

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
---
 scripts/checkpatch.pl | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d40403..96138d6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2643,20 +2643,20 @@ sub process {
 			$long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
 			$space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
 			$case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
-			if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
+			if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.*)"\)/i) {
 				$orig_desc = $1;
 				$hasparens = 1;
 			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
 				 defined $rawlines[$linenr] &&
-				 $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
+				 $rawlines[$linenr] =~ /^\s*\("(.*)"\)/) {
 				$orig_desc = $1;
 				$hasparens = 1;
-			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
-				 defined $rawlines[$linenr] &&
-				 $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
-				$line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
+		       } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".*$/i &&
+				defined $rawlines[$linenr] &&
+				$rawlines[$linenr] =~ /^\s*.*"\)/) {
+				$line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.*)$/i;
 				$orig_desc = $1;
-				$rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
+				$rawlines[$linenr] =~ /^\s*(.*)"\)/;
 				$orig_desc .= " " . $1;
 				$hasparens = 1;
 			}
-- 
2.7.4

Re: [PATCH] checkpatch: relax check for revert commit

[Jia He]

Hi Andy & Joe

Although it is minor, it is a real bug, I thought.

Is there any comment? Thank you

---

Cheers,

Jia


On 4/2/2018 10:28 PM, Jia He Wrote:
> For revert commit, it might has two double quotation marks in its
> commit log.
>
> Relax the check condition for revert commit to avoid checkpatch
> errors.
>
> Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> ---
>   scripts/checkpatch.pl | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3d40403..96138d6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2643,20 +2643,20 @@ sub process {
>   			$long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
>   			$space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
>   			$case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> -			if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
> +			if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.*)"\)/i) {
>   				$orig_desc = $1;
>   				$hasparens = 1;
>   			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
>   				 defined $rawlines[$linenr] &&
> -				 $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
> +				 $rawlines[$linenr] =~ /^\s*\("(.*)"\)/) {
>   				$orig_desc = $1;
>   				$hasparens = 1;
> -			} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
> -				 defined $rawlines[$linenr] &&
> -				 $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
> -				$line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
> +		       } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".*$/i &&
> +				defined $rawlines[$linenr] &&
> +				$rawlines[$linenr] =~ /^\s*.*"\)/) {
> +				$line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.*)$/i;
>   				$orig_desc = $1;
> -				$rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
> +				$rawlines[$linenr] =~ /^\s*(.*)"\)/;
>   				$orig_desc .= " " . $1;
>   				$hasparens = 1;
>   			}

Re: [PATCH] checkpatch: relax check for revert commit

[Joe Perches]

On Sun, 2018-04-08 at 10:12 +0800, Jia He wrote:
> Hi Andy & Joe
> 
> Although it is minor, it is a real bug, I thought.
> 
> Is there any comment? Thank you

If you really want to do something useful here,
quote the quote characters and compare for that.

Re: [PATCH] checkpatch: relax check for revert commit

[Jia He]

On 4/9/2018 1:07 AM, Joe Perches Wrote:
> On Sun, 2018-04-08 at 10:12 +0800, Jia He wrote:
>> Hi Andy & Joe
>>
>> Although it is minor, it is a real bug, I thought.
>>
>> Is there any comment? Thank you
> If you really want to do something useful here,
> quote the quote characters and compare for that.
>
>
Hi Joe, thanks for the comments
I am not sure I understand you comments here.
e.g.If there is such a commit f5c1350256fb ("Revert "mm/page_alloc: fix 
memmap_init_zone pageblock alignment"") in the decription log.

Do you want me to quote the first match - Revert "mm/page_alloc: fix 
memmap_init_zone pageblock alignment" or the second one - mm/page_alloc: fix 
memmap_init_zone pageblock alignment ?
Thanks for more details

-- 
Cheers,
Jia

Re: [PATCH] checkpatch: relax check for revert commit

[Joe Perches]

On Mon, 2018-04-09 at 14:37 +0800, Jia He wrote:
> 
> On 4/9/2018 1:07 AM, Joe Perches Wrote:
> > On Sun, 2018-04-08 at 10:12 +0800, Jia He wrote:
> > > Hi Andy & Joe
> > > 
> > > Although it is minor, it is a real bug, I thought.
> > > 
> > > Is there any comment? Thank you
> > 
> > If you really want to do something useful here,
> > quote the quote characters and compare for that.
> > 
> > 
> 
> Hi Joe, thanks for the comments
> I am not sure I understand you comments here.

Change the tests to allow quote characters instead
of ignoring them.

To do this matching, any internal  quote character in
the actual commit string and the commit string from the
patch must be perl quoted with \Q<string>\E so that the
expected title can be matched properly.

Re: [PATCH] checkpatch: relax check for revert commit

[Jia He]

On 4/9/2018 6:50 PM, Joe Perches Wrote:
> On Mon, 2018-04-09 at 14:37 +0800, Jia He wrote:
>> On 4/9/2018 1:07 AM, Joe Perches Wrote:
>>> On Sun, 2018-04-08 at 10:12 +0800, Jia He wrote:
>>>> Hi Andy & Joe
>>>>
>>>> Although it is minor, it is a real bug, I thought.
>>>>
>>>> Is there any comment? Thank you
>>> If you really want to do something useful here,
>>> quote the quote characters and compare for that.
>>>
>>>
>> Hi Joe, thanks for the comments
>> I am not sure I understand you comments here.
> Change the tests to allow quote characters instead
> of ignoring them.
>
> To do this matching, any internal  quote character in
> the actual commit string and the commit string from the
> patch must be perl quoted with \Q<string>\E so that the
> expected title can be matched properly.
Sorry,I didn't catch your point, even can't image how \Q \E is helpful. ;-)
Maybe I haven't explained my changes clearly.
e.g. commit f5c1350256fb ("Revert "XXX"")
2617                         } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
2618                                  defined $rawlines[$linenr] &&
2619                                  $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
2620                                 $orig_desc = $1;
2621                                 $hasparens = 1;
line 2619, the match pattern ([^"]+)"\) will match NULL and let $orig_desc="" in 
this case.
My change is to let $orig_desc to be Revert "XXX"
Since the old code will check the commit id by git command to get the _real_ 
commit title.
I can't understand why I should use \Q \E here, sorry if I am missing something.

Cheers,
Jia