Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/3] checkpatch: skip spacing tests on linker scripts | From | Joe Perches <> | Date | Wed, 30 Jun 2021 10:12:42 -0700 |
| |
On Wed, 2021-06-30 at 10:38 -0600, jim.cromie@gmail.com wrote: > On Tue, Jun 29, 2021 at 2:01 PM Joe Perches <joe@perches.com> wrote: > > > > On Tue, 2021-06-29 at 13:50 -0600, jim.cromie@gmail.com wrote: > > > this does 3 different things > > > > > > - non-capturing matches - these add no functionality, > > > > true, it's nominally a bit faster through. > > > > > - moves the skip-remaining-tests check after SPDX > > > that feels like a legal Q: should it be on all files ? > > > moving it does seem proper though. > > > > to me too. > > > > > - adds the skip linker-script > > > since i went ahead and added it 3 times to see errs/warns > > > I didnt consider your precise placement, > > > how does it do with 18/8 errs/warnings on ref-test ? > > > > $ ./scripts/checkpatch.pl -f include/asm-generic/vmlinux.lds.h --strict --terse > > cool options. > <Aside> > some oddities are hidden there; > Im seeing the err/warn counts change along with use of those options. > not a big deal, but it is mildly surprising > forex: > $ scripts/checkpatch.pl -f include/asm-generic/vmlinux.lds.h --terse > ... > total: 18 errors, 7 warnings, 1164 lines checked > $ scripts/checkpatch.pl -f include/asm-generic/vmlinux.lds.h --terse --strict > ... > total: 9 errors, 7 warnings, 95 checks, 1164 lines checked
The difference is a --strict test 'if ($check)' that precedes and in effect 'overrides' the ERROR output for that output around line 5120.
$ ./scripts/checkpatch.pl -f include/asm-generic/vmlinux.lds.h --terse [] include/asm-generic/vmlinux.lds.h:101: ERROR: need consistent spacing around '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: ERROR: need consistent spacing around '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: ERROR: need consistent spacing around '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: ERROR: need consistent spacing around '*' (ctx:VxW)
vs:
$ ./scripts/checkpatch.pl -f include/asm-generic/vmlinux.lds.h --terse --strict [] include/asm-generic/vmlinux.lds.h:101: CHECK: spaces preferred around that '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: CHECK: spaces preferred around that '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: CHECK: spaces preferred around that '*' (ctx:VxW) include/asm-generic/vmlinux.lds.h:101: CHECK: spaces preferred around that '*' (ctx:VxW)
> just to note, this is about a generalization of > > commit 263afd39c06f5939ef943e0d535380d4b8e56484 > Author: Chris Down <chris@chrisdown.name> > Date: Thu Feb 25 17:22:04 2021 -0800 > > checkpatch: don't warn about colon termination in linker scripts
Which means the additional test in that commit should be removed too.
Maybe: --- scripts/checkpatch.pl | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 461d4221e4a4a..f4f5826054214 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3617,9 +3617,6 @@ sub process { "It's generally not useful to have the filename in the file\n" . $herecurr); } -# check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); - # check for using SPDX-License-Identifier on the wrong line number if ($realline != $checklicenseline && $rawline =~ /\bSPDX-License-Identifier:/ && @@ -3628,6 +3625,9 @@ sub process { "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); } +# check we are in a valid source file if not then ignore this hunk + next if ($realfile !~ /\.(?:h|c|s|S|sh|dtsi|dts)$/); + # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -3708,8 +3708,8 @@ sub process { "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr); } -# check we are in a valid source file C or perl if not then ignore this hunk - next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/); +# check we are in a valid source C or .dts? file, if not then ignore this hunk + next if ($realfile !~ /\.(?:h|c|dtsi|dts)$/); # at the beginning of a line any tabs must come first and anything # more than $tabsize must use tabs. @@ -3737,6 +3737,9 @@ sub process { } } +# skip all following test for linker files. + next if ($realfile =~ /\.lds\.h$/); + # check for assignments on the start of a line if ($sline =~ /^\+\s+($Assignment)[^=]/) { my $operator = $1; @@ -3970,7 +3973,7 @@ sub process { } # check we are in a valid C source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c)$/); + next if ($realfile !~ /\.(?:h|c)$/); # check for unusual line ending [ or ( if ($line =~ /^\+.*([\[\(])\s*$/) { @@ -5147,7 +5150,7 @@ sub process { # A colon needs no spaces before when it is # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./ and $realfile !~ m@.*\.lds\.h$@) { + if ($ctx =~ /Wx./) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
| |