[Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style

Alex Williamson posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171003220038.16762.20147.stgit@gimli.home
Test checkpatch passed
Test docker passed
Test s390x passed
scripts/checkpatch.pl |   19 -------------------
1 file changed, 19 deletions(-)
[Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Alex Williamson 6 years, 5 months ago
Commit c3e5875afc0f ("checkpatch: check trace-events code style")
introduces a regression as reported:

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html

Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
Execution of ./scripts/checkpatch.pl aborted due to compilation errors.

$ perl -v

This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi

As no fix or discussion has resulted, revert the original patch.

Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Fixes: c3e5875afc0f ("checkpatch: check trace-events code style")
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 scripts/checkpatch.pl |   19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3c0a28e644aa..f7e785d12a49 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1422,25 +1422,6 @@ sub process {
 			$rpt_cleaners = 1;
 		}
 
-# checks for trace-events files
-		if ($realfile =~ /trace-events$/ && $line =~ /^\+/) {
-			if ($rawline =~ /%[-+ 0]*#/) {
-				ERROR("Don't use '#' flag of printf format ('%#') in " .
-				      "trace-events, use '0x' prefix instead\n" . $herecurr);
-			} else {
-				my $hex =
-					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
-
-				# don't consider groups splitted by [.:/ ], like 2A.20:12ab
-				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
-
-				if ($tmpline =~ /(?<!0x)$hex/) {
-					ERROR("Hex numbers must be prefixed with '0x'\n" .
-					      $herecurr);
-				}
-			}
-		}
-
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
 


Re: [Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Vladimir Sementsov-Ogievskiy 6 years, 5 months ago
04.10.2017 01:00, Alex Williamson wrote:
> Commit c3e5875afc0f ("checkpatch: check trace-events code style")
> introduces a regression as reported:
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html
>
> Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> Execution of ./scripts/checkpatch.pl aborted due to compilation errors.
>
> $ perl -v
>
> This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi
>
> As no fix or discussion has resulted, revert the original patch.

However, this revert will be a regression too - checkpatch will not 
check that thing more.

>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Fixes: c3e5875afc0f ("checkpatch: check trace-events code style")
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>   scripts/checkpatch.pl |   19 -------------------
>   1 file changed, 19 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3c0a28e644aa..f7e785d12a49 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1422,25 +1422,6 @@ sub process {
>   			$rpt_cleaners = 1;
>   		}
>   
> -# checks for trace-events files
> -		if ($realfile =~ /trace-events$/ && $line =~ /^\+/) {
> -			if ($rawline =~ /%[-+ 0]*#/) {
> -				ERROR("Don't use '#' flag of printf format ('%#') in " .
> -				      "trace-events, use '0x' prefix instead\n" . $herecurr);
> -			} else {
> -				my $hex =
> -					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
> -
> -				# don't consider groups splitted by [.:/ ], like 2A.20:12ab
> -				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
> -
> -				if ($tmpline =~ /(?<!0x)$hex/) {
> -					ERROR("Hex numbers must be prefixed with '0x'\n" .
> -					      $herecurr);
> -				}
> -			}
> -		}
> -
>   # check we are in a valid source file if not then ignore this hunk
>   		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
>   
>


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Vladimir Sementsov-Ogievskiy 6 years, 5 months ago
04.10.2017 01:00, Alex Williamson wrote:
> Commit c3e5875afc0f ("checkpatch: check trace-events code style")
> introduces a regression as reported:
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html
>
> Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> Execution of ./scripts/checkpatch.pl aborted due to compilation errors.
>
> $ perl -v
>
> This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi

v5.10.1 is 8 years old.
most possibly the error relates to  /r modifier, which was added int 
perl 5.14, so, possible solution would be

@@ -1432,7 +1432,8 @@ sub process {
                      qr/%[-+ 
*.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;

                  # don't consider groups splitted by [.:/ ], like 
2A.20:12ab
-                my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
+                my $tmpline = $rawline;
+                my $tmpline =~ s/($hex[.:\/ ])+$hex//g;

                  if ($tmpline =~ /(?<!0x)$hex/) {
                      ERROR("Hex numbers must be prefixed with '0x'\n" .


>
> As no fix or discussion has resulted, revert the original patch.

Sorry for no answer on your report

>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Fixes: c3e5875afc0f ("checkpatch: check trace-events code style")
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>   scripts/checkpatch.pl |   19 -------------------
>   1 file changed, 19 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3c0a28e644aa..f7e785d12a49 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1422,25 +1422,6 @@ sub process {
>   			$rpt_cleaners = 1;
>   		}
>   
> -# checks for trace-events files
> -		if ($realfile =~ /trace-events$/ && $line =~ /^\+/) {
> -			if ($rawline =~ /%[-+ 0]*#/) {
> -				ERROR("Don't use '#' flag of printf format ('%#') in " .
> -				      "trace-events, use '0x' prefix instead\n" . $herecurr);
> -			} else {
> -				my $hex =
> -					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
> -
> -				# don't consider groups splitted by [.:/ ], like 2A.20:12ab
> -				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
> -
> -				if ($tmpline =~ /(?<!0x)$hex/) {
> -					ERROR("Hex numbers must be prefixed with '0x'\n" .
> -					      $herecurr);
> -				}
> -			}
> -		}
> -
>   # check we are in a valid source file if not then ignore this hunk
>   		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
>   
>


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Daniel P. Berrange 6 years, 5 months ago
On Wed, Oct 04, 2017 at 12:59:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 04.10.2017 01:00, Alex Williamson wrote:
> > Commit c3e5875afc0f ("checkpatch: check trace-events code style")
> > introduces a regression as reported:
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html
> > 
> > Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> > syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> > Execution of ./scripts/checkpatch.pl aborted due to compilation errors.
> > 
> > $ perl -v
> > 
> > This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi
> 
> v5.10.1 is 8 years old.
> most possibly the error relates to  /r modifier, which was added int perl
> 5.14, so, possible solution would be
> 
> @@ -1432,7 +1432,8 @@ sub process {
>                      qr/%[-+
> *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
> 
>                  # don't consider groups splitted by [.:/ ], like 2A.20:12ab
> -                my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
> +                my $tmpline = $rawline;
> +                my $tmpline =~ s/($hex[.:\/ ])+$hex//g;
> 
>                  if ($tmpline =~ /(?<!0x)$hex/) {
>                      ERROR("Hex numbers must be prefixed with '0x'\n" .

Yes, that fixes the error when run on a rhel6 vintage Perl. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Alex Williamson 6 years, 5 months ago
On Wed, 4 Oct 2017 12:59:04 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> 04.10.2017 01:00, Alex Williamson wrote:
> > Commit c3e5875afc0f ("checkpatch: check trace-events code style")
> > introduces a regression as reported:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html
> >
> > Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> > syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
> > Execution of ./scripts/checkpatch.pl aborted due to compilation errors.
> >
> > $ perl -v
> >
> > This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi  
> 
> v5.10.1 is 8 years old.
> most possibly the error relates to  /r modifier, which was added int 
> perl 5.14, so, possible solution would be
> 
> @@ -1432,7 +1432,8 @@ sub process {
>                       qr/%[-+ 
> *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
> 
>                   # don't consider groups splitted by [.:/ ], like 
> 2A.20:12ab
> -                my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
> +                my $tmpline = $rawline;
> +                my $tmpline =~ s/($hex[.:\/ ])+$hex//g;
> 
>                   if ($tmpline =~ /(?<!0x)$hex/) {
>                       ERROR("Hex numbers must be prefixed with '0x'\n" .

Yes, that fixes it.

> >
> > As no fix or discussion has resulted, revert the original patch.  
> 
> Sorry for no answer on your report

No worries, this seems to have gotten the job done.  Please post the
patch above and I withdraw my revert.  Thanks,

Alex

> >
> > Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Fixes: c3e5875afc0f ("checkpatch: check trace-events code style")
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >   scripts/checkpatch.pl |   19 -------------------
> >   1 file changed, 19 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 3c0a28e644aa..f7e785d12a49 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -1422,25 +1422,6 @@ sub process {
> >   			$rpt_cleaners = 1;
> >   		}
> >   
> > -# checks for trace-events files
> > -		if ($realfile =~ /trace-events$/ && $line =~ /^\+/) {
> > -			if ($rawline =~ /%[-+ 0]*#/) {
> > -				ERROR("Don't use '#' flag of printf format ('%#') in " .
> > -				      "trace-events, use '0x' prefix instead\n" . $herecurr);
> > -			} else {
> > -				my $hex =
> > -					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
> > -
> > -				# don't consider groups splitted by [.:/ ], like 2A.20:12ab
> > -				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
> > -
> > -				if ($tmpline =~ /(?<!0x)$hex/) {
> > -					ERROR("Hex numbers must be prefixed with '0x'\n" .
> > -					      $herecurr);
> > -				}
> > -			}
> > -		}
> > -
> >   # check we are in a valid source file if not then ignore this hunk
> >   		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
> >   
> >  
> 
> 


Re: [Qemu-devel] [PATCH] Revert: checkpatch: check trace-events code style
Posted by Vladimir Sementsov-Ogievskiy 6 years, 5 months ago
04.10.2017 17:59, Alex Williamson wrote:
> On Wed, 4 Oct 2017 12:59:04 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> 04.10.2017 01:00, Alex Williamson wrote:
>>> Commit c3e5875afc0f ("checkpatch: check trace-events code style")
>>> introduces a regression as reported:
>>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg05820.html
>>>
>>> Bareword found where operator expected at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
>>> syntax error at ./scripts/checkpatch.pl line 1350, near "s/($hex[.:\/ ])+$hex//gr"
>>> Execution of ./scripts/checkpatch.pl aborted due to compilation errors.
>>>
>>> $ perl -v
>>>
>>> This is perl, v5.10.1 (*) built for x86_64-linux-thread-multi
>> v5.10.1 is 8 years old.
>> most possibly the error relates to  /r modifier, which was added int
>> perl 5.14, so, possible solution would be
>>
>> @@ -1432,7 +1432,8 @@ sub process {
>>                        qr/%[-+
>> *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
>>
>>                    # don't consider groups splitted by [.:/ ], like
>> 2A.20:12ab
>> -                my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
>> +                my $tmpline = $rawline;
>> +                my $tmpline =~ s/($hex[.:\/ ])+$hex//g;

of course, last line should be without "my". I've sent a fixed patch.

>>
>>                    if ($tmpline =~ /(?<!0x)$hex/) {
>>                        ERROR("Hex numbers must be prefixed with '0x'\n" .
> Yes, that fixes it.
>
>>> As no fix or discussion has resulted, revert the original patch.
>> Sorry for no answer on your report
> No worries, this seems to have gotten the job done.  Please post the
> patch above and I withdraw my revert.  Thanks,
>
> Alex
>
>>> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Fixes: c3e5875afc0f ("checkpatch: check trace-events code style")
>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>> ---
>>>    scripts/checkpatch.pl |   19 -------------------
>>>    1 file changed, 19 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 3c0a28e644aa..f7e785d12a49 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -1422,25 +1422,6 @@ sub process {
>>>    			$rpt_cleaners = 1;
>>>    		}
>>>    
>>> -# checks for trace-events files
>>> -		if ($realfile =~ /trace-events$/ && $line =~ /^\+/) {
>>> -			if ($rawline =~ /%[-+ 0]*#/) {
>>> -				ERROR("Don't use '#' flag of printf format ('%#') in " .
>>> -				      "trace-events, use '0x' prefix instead\n" . $herecurr);
>>> -			} else {
>>> -				my $hex =
>>> -					qr/%[-+ *.0-9]*([hljztL]|ll|hh)?(x|X|"\s*PRI[xX][^"]*"?)/;
>>> -
>>> -				# don't consider groups splitted by [.:/ ], like 2A.20:12ab
>>> -				my $tmpline = $rawline =~ s/($hex[.:\/ ])+$hex//gr;
>>> -
>>> -				if ($tmpline =~ /(?<!0x)$hex/) {
>>> -					ERROR("Hex numbers must be prefixed with '0x'\n" .
>>> -					      $herecurr);
>>> -				}
>>> -			}
>>> -		}
>>> -
>>>    # check we are in a valid source file if not then ignore this hunk
>>>    		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
>>>    
>>>   
>>


-- 
Best regards,
Vladimir