diff options
author | Elyes Haouas <ehaouas@noos.fr> | 2022-05-22 11:11:11 +0200 |
---|---|---|
committer | Felix Held <felix-coreboot@felixheld.de> | 2022-05-24 12:57:48 +0000 |
commit | 26e0b9461405a26eda786eeffd617253f76692a3 (patch) | |
tree | 51ff50a6dd24612c9348f78bc1dfa3fbc039c7b2 | |
parent | f04426488694c68bdde817c1f1a94e6ba16fb701 (diff) |
util/lint/checkpatch.pl: Reduce difference with linux v5.18
Signed-off-by: Elyes Haouas <ehaouas@noos.fr>
Change-Id: Id5eb4823399088746a34721a9855bbaf5f97b7b6
Reviewed-on: https://review.coreboot.org/c/coreboot/+/64572
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <gaumless@tutanota.com>
-rwxr-xr-x | util/lint/checkpatch.pl | 131 |
1 files changed, 82 insertions, 49 deletions
diff --git a/util/lint/checkpatch.pl b/util/lint/checkpatch.pl index 6d2d1dd011..1bf9e6556d 100755 --- a/util/lint/checkpatch.pl +++ b/util/lint/checkpatch.pl @@ -480,9 +480,10 @@ our $Sparse = qr{ __force| __iomem| __must_check| - __init_refok| __kprobes| __ref| + __refconst| + __refdata| __rcu| __private }x; @@ -496,6 +497,7 @@ our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeIni # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -512,12 +514,14 @@ our $Attribute = qr{ __noclone| __deprecated| __read_mostly| + __ro_after_init| __kprobes| $InitAttribute| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp| - __weak + __weak| + __alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\) }x; our $Modifier; our $Inline = qr{inline|__always_inline|noinline|__inline|__inline__}; @@ -529,7 +533,7 @@ our $Binary = qr{(?i)0b[01]+$Int_type?}; our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; our $Int = qr{[0-9]+$Int_type?}; our $Octal = qr{0[0-7]+$Int_type?}; -our $String = qr{"[X\t]*"}; +our $String = qr{(?:\b[Lu])?"[X\t]*"}; our $Float_hex = qr{(?i)0x[0-9a-f]+p-?[0-9]+[fl]?}; our $Float_dec = qr{(?i)(?:[0-9]+\.[0-9]*|[0-9]*\.[0-9]+)(?:e-?[0-9]+)?[fl]?}; our $Float_int = qr{(?i)[0-9]+e-?[0-9]+[fl]?}; @@ -861,12 +865,12 @@ if (show_type("CONST_STRUCT")) { or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; } -my $typeOtherTypedefs = ""; -if (length($typedefsfile)) { +if (defined($typedefsfile)) { + my $typeOtherTypedefs; read_words(\$typeOtherTypedefs, $typedefsfile) or warn "No additional types will be considered - file '$typedefsfile': $!\n"; + $typeTypedefs .= '|' . $typeOtherTypedefs if (defined $typeOtherTypedefs); } -$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne ""); sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; @@ -905,12 +909,12 @@ sub build_types { }x; $Type = qr{ $NonptrType - (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)? + (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+){0,4} (?:\s+$Inline|\s+$Modifier)* }x; $TypeMisordered = qr{ $NonptrTypeMisordered - (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)? + (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+){0,4} (?:\s+$Inline|\s+$Modifier)* }x; $Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type}; @@ -931,7 +935,6 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)}; our $declaration_macros = qr{(?x: (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(| - (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(| (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\( )}; @@ -1061,7 +1064,7 @@ sub git_commit_info { return ($id, $desc) if ($#lines < 0); - if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous\./) { + if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous/) { # Maybe one day convert this block of bash into something that returns # all matching commit ids, but it's very slow... # @@ -1071,7 +1074,8 @@ sub git_commit_info { # git log --format='%H %s' -1 $line | # echo "commit $(cut -c 1-12,41-)" # done - } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree\./) { + } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree\./ || + $lines[0] =~ /^fatal: bad object $commit/) { $id = undef; } else { $id = substr($lines[0], 0, 12); @@ -1333,7 +1337,7 @@ sub copy_spacing { sub line_stats { my ($line) = @_; - # Drop the diff line leader + # Drop the diff line leader and expand tabs $line =~ s/^.//; # Treat labels like whitespace when counting indentation @@ -1745,8 +1749,16 @@ sub ctx_statement_level { sub ctx_locate_comment { my ($first_line, $end_line) = @_; + # If c99 comment on the current line, or the line before or after + my ($current_comment) = ($rawlines[$end_line - 1] =~ m@^\+.*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line - 2] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + # Catch a comment on the end of the line itself. - my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); + ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); return $current_comment if (defined $current_comment); # Look through the context and try and figure out if there is a @@ -2429,6 +2441,7 @@ sub process { my $in_commit_log = 0; #Scanning lines before patch my $has_commit_log = 0; #Encountered lines before patch my $commit_log_possible_stack_dump = 0; + my $commit_log_long_line = 0; my $commit_log_has_diff = 0; my $reported_maintainer_file = 0; my $non_utf8_charset = 0; @@ -2698,8 +2711,8 @@ sub process { # Check if the commit log has what seems like a diff which can confuse patch if ($in_commit_log && !$commit_log_has_diff && - (($line =~ m@^\s+diff\b.*a/[\w/]+@ && - $line =~ m@^\s+diff\b.*a/([\w/]+)\s+b/$1\b@) || + (($line =~ m@^\s+diff\b.*a/([\w/]+)@ && + $line =~ m@^\s+diff\b.*a/[\w/]+\s+b/$1\b@) || $line =~ m@^\s*(?:\-\-\-\s+a/|\+\+\+\s+b/)@ || $line =~ m/^\s*\@\@ \-\d+,\d+ \+\d+,\d+ \@\@/)) { ERROR("DIFF_IN_COMMIT_MSG", @@ -2817,8 +2830,10 @@ sub process { ($line =~ /^\s*(?:WARNING:|BUG:)/ || $line =~ /^\s*\[\s*\d+\.\d{6,6}\s*\]/ || # timestamp - $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/)) { - # stack dump address + $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/) || + $line =~ /^(?:\s+\w+:\s+[0-9a-fA-F]+){3,3}/ || + $line =~ /^\s*\#\d+\s*\[[0-9a-fA-F]+\]\s*\w+ at [0-9a-fA-F]+/) { + # stack dump address styles $commit_log_possible_stack_dump = 1; } @@ -2834,6 +2849,7 @@ sub process { $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 72 chars per line)\n" . $herecurr); + $commit_log_long_line = 1; } # coreboot: The line subject limit is 65 @@ -2986,15 +3002,18 @@ sub process { # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) { + while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $typo = $1; + my $blank = copy_spacing($rawline); + my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo); + my $hereptr = "$hereline$ptr\n"; my $typo_fix = $spelling_fix{lc($typo)}; $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("TYPO_SPELLING", - "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $hereptr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; } @@ -3581,12 +3600,13 @@ sub process { } # check indentation of a line with a break; -# if the previous line is a goto or return and is indented the same # of tabs +# if the previous line is a goto, return or break +# and is indented the same # of tabs if ($sline =~ /^\+([\t]+)break\s*;\s*$/) { my $tabs = $1; - if ($prevline =~ /^\+$tabs(?:goto|return)\b/) { + if ($prevline =~ /^\+$tabs(?:goto|return|break)\b/) { WARN("UNNECESSARY_BREAK", - "break is not useful after a goto or return\n" . $hereprev); + "break is not useful after a $1\n" . $hereprev); } } @@ -4035,6 +4055,7 @@ sub process { # XXX(foo); # EXPORT_SYMBOL(something_foo); my $name = $1; + $name =~ s/^\s*($Ident).*/$1/; if ($stat =~ /^(?:.\s*}\s*\n)?.([A-Z_]+)\s*\(\s*($Ident)/ && $name =~ /^${Ident}_$2/) { #print "FOO C name<$name>\n"; @@ -4097,7 +4118,7 @@ sub process { WARN("STATIC_CONST_CHAR_ARRAY", "static const char * array should probably be static const char * const\n" . $herecurr); - } + } # check for static char foo[] = "bar" declarations. if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/) { @@ -4139,7 +4160,7 @@ sub process { } # check for function declarations without arguments like "int foo()" - if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) { + if ($line =~ /(\b$Type\s*$Ident)\s*\(\s*\)/) { if (ERROR("FUNCTION_WITHOUT_ARGS", "Bad function definition - $1() should probably be $1(void)\n" . $herecurr) && $fix) { @@ -4287,7 +4308,7 @@ sub process { $fix) { fix_delete_line($fixlinenr, $rawline); my $fixed_line = $rawline; - $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*){(.*)$/; + $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*)\{(.*)$/; my $line1 = $1; my $line2 = $2; fix_insert_line($fixlinenr, ltrim($line1)); @@ -4795,12 +4816,12 @@ sub process { #need space before brace following if, while, etc if (($line =~ /\(.*\)\{/ && $line !~ /\($Type\)\{/) || - $line =~ /do\{/) { + $line =~ /\b(?:else|do)\{/) { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { #coreboot - Open braces must be escaped in regex - $fixed[$fixlinenr] =~ s/^(\+.*(?:do|\)))\{/$1 \{/; + $fixed[$fixlinenr] =~ s/^(\+.*(?:do|else|\)))\{/$1 \{/; } } @@ -4814,7 +4835,7 @@ sub process { # closing brace should have a space following it when it has anything # on the line - if ($line =~ /}(?!(?:,|;|\)))\S/) { + if ($line =~ /}(?!(?:,|;|\)|\}))\S/) { if (ERROR("SPACING", "space required after that close brace '}'\n" . $herecurr) && $fix) { @@ -4914,9 +4935,13 @@ sub process { } } -#goto labels aren't indented, allow a single space however - if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and - !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { +# check that goto labels aren't indented (allow a single space indentation) +# and ignore bitfield definitions like foo:1 +# Strictly, labels can have whitespace after the identifier and before the : +# but this is not allowed here as many ?: uses would appear to be labels + if ($sline =~ /^.\s+[A-Za-z_][A-Za-z\d_]*:(?!\s*\d+)/ && + $sline !~ /^. [A-Za-z\d_][A-Za-z\d_]*:/ && + $sline !~ /^.\s+default:/) { if (WARN("INDENTED_LABEL", "labels should not be indented\n" . $herecurr) && $fix) { @@ -5180,8 +5205,9 @@ sub process { $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && #Ignore Page<foo> variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && -#Ignore SI style variants like nS, mV and dB (ie: max_uV, regulator_min_uA_show) - $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/ && +#Ignore SI style variants like nS, mV and dB +#(ie: max_uV, regulator_min_uA_show, RANGE_mA_VALUE) + $var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ && #Ignore some three character SI units explicitly, like MiB and KHz $var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) { while ($var =~ m{($Ident)}g) { @@ -5262,6 +5288,7 @@ sub process { if (defined $define_args && $define_args ne "") { $define_args = substr($define_args, 1, length($define_args) - 2); $define_args =~ s/\s*//g; + $define_args =~ s/\\\+?//g; @def_args = split(",", $define_args); } @@ -5271,9 +5298,9 @@ sub process { $dstat =~ s/\s*$//s; # Flatten any parentheses and braces - while ($dstat =~ s/\([^\(\)]*\)/1/ || - $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/.\[[^\[\]]*\]/1/) + while ($dstat =~ s/\([^\(\)]*\)/1u/ || + $dstat =~ s/\{[^\{\}]*\}/1u/ || + $dstat =~ s/.\[[^\[\]]*\]/1u/) { } @@ -5314,6 +5341,7 @@ sub process { $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) + $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ && # while (...) {...} $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... @@ -5355,7 +5383,7 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp_stmt =~ s/\#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*\#\#//g; my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; @@ -5687,9 +5715,14 @@ sub process { # warn about #if 0 if ($line =~ /^.\s*\#\s*if\s+0\b/) { - CHK("REDUNDANT_CODE", - "if this code is redundant consider removing it\n" . - $herecurr); + WARN("IF_0", + "Consider removing the code enclosed by this #if 0 and its #endif\n" . $herecurr); + } + +# warn about #if 1 + if ($line =~ /^.\s*\#\s*if\s+1\b/) { + WARN("IF_1", + "Consider removing the #if 1 and its #endif\n" . $herecurr); } # check for needless "if (<foo>) fn(<foo>)" uses @@ -5917,8 +5950,7 @@ sub process { my $barriers = qr{ mb| rmb| - wmb| - read_barrier_depends + wmb }x; my $barrier_stems = qr{ mb__before_atomic| @@ -5959,10 +5991,12 @@ sub process { } } -# check for smp_read_barrier_depends and read_barrier_depends - if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) { - WARN("READ_BARRIER_DEPENDS", - "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr); +# check for data_race without a comment. + if ($line =~ /\bdata_race\s*\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("DATA_RACE", + "data_race without comment\n" . $herecurr); + } } # check of hardware specific defines @@ -6327,8 +6361,7 @@ sub process { if (defined $cond) { substr($s, 0, length($cond), ''); } - if ($s =~ /^\s*;/ && - $function_name ne 'uninitialized_var') + if ($s =~ /^\s*;/) { WARN("AVOID_EXTERNS", "externs should be avoided in .c files\n" . $herecurr); @@ -6402,7 +6435,7 @@ sub process { # alloc style # p = alloc(sizeof(struct foo), ...) should be p = alloc(sizeof(*p), ...) if ($perl_version_ok && - $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*([kv][mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { + $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { CHK("ALLOC_SIZEOF_STRUCT", "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } |