History

checkpatch: fix CONST_STRUCT when const_structs.checkpatch is missing

Checkpatch reports warnings when some specific structs are not declared as const in the code. The list of structs to consider was initially defined in the checkpatch.pl script itself, but it was later moved to an external file (scripts/const_structs.checkpatch), in commit bf1fa1dae68e ("checkpatch: externalize the structs that should be const"). This introduced two minor issues: - When file scripts/const_structs.checkpatch is not present (for example, if checkpatch is run outside of the kernel directory with the "--no-tree" option), a warning is printed to stderr to tell the user that "No structs that should be const will be found". This is fair, but the warning is printed unconditionally, even if the option "--ignore CONST_STRUCT" is passed. In the latter case, we explicitly ask checkpatch to skip this check, so no warning should be printed. - When scripts/const_structs.checkpatch is missing, or even when trying to silence the warning by adding an empty file, $const_structs is set to "", and the regex used for finding structs that should be const, "$line =~ /struct\s+($const_structs)(?!\s*\{)/)", matches all structs found in the code, thus reporting a number of false positives. Let's fix the first item by skipping scripts/const_structs.checkpatch processing if "CONST_STRUCT" checks are ignored, and the second one by skipping the test if $const_structs is not defined. Since we modify the read_words() function a little bit, update the checks for $typedefsfile/$typeOtherTypedefs as well.

Quentin Monnet committed 6 years ago

Commit ced69da1 parent 65b64b31 file · +12 −9Browse files

Showing the changes to scripts (1 of 1 file). Show all

Unified · Split

scripts/checkpatch.pl+12−9
@@ -59,7 +59,7 @@ my $spelling_file = "$D/spelling.txt"; · 58 unmodified lines
59my $codespell = 0;59my $codespell = 0;
60my $codespellfile = "/usr/share/codespell/dictionary.txt";60my $codespellfile = "/usr/share/codespell/dictionary.txt";
61my $conststructsfile = "$D/const_structs.checkpatch";61my $conststructsfile = "$D/const_structs.checkpatch";
62my $typedefsfile = "";62my $typedefsfile;
63my $color = "auto";63my $color = "auto";
64my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE64my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
65# git output parsing needs US English output, so first set backtick child process LANGUAGE65# git output parsing needs US English output, so first set backtick child process LANGUAGE
@@ -756,7 +756,7 @@ sub read_words { · 690 unmodified lines
756 next;756 next;
757 }757 }
758 758
759 $$wordsRef .= '|' if ($$wordsRef ne "");759 $$wordsRef .= '|' if (defined $$wordsRef);
760 $$wordsRef .= $line;760 $$wordsRef .= $line;
761 }761 }
762 close($file);762 close($file);
@@ -766,16 +766,18 @@ sub read_words { · 3 unmodified lines
766 return 0;766 return 0;
767}767}
768 768
769my $const_structs = "";769my $const_structs;
770read_words(\$const_structs, $conststructsfile)770if (show_type("CONST_STRUCT")) {
771 or warn "No structs that should be const will be found - file '$conststructsfile': $!\n";771 read_words(\$const_structs, $conststructsfile)
772 or warn "No structs that should be const will be found - file '$conststructsfile': $!\n";
773}
772 774
773my $typeOtherTypedefs = "";775if (defined($typedefsfile)) {
774if (length($typedefsfile)) {776 my $typeOtherTypedefs;
775 read_words(\$typeOtherTypedefs, $typedefsfile)777 read_words(\$typeOtherTypedefs, $typedefsfile)
776 or warn "No additional types will be considered - file '$typedefsfile': $!\n";778 or warn "No additional types will be considered - file '$typedefsfile': $!\n";
779 $typeTypedefs .= '|' . $typeOtherTypedefs if (defined $typeOtherTypedefs);
777}780}
778$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");
779 781
780sub build_types {782sub build_types {
781 my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)";783 my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)";
@@ -6643,7 +6645,8 @@ sub process { · 5861 unmodified lines
6643 6645
6644# check for various structs that are normally const (ops, kgdb, device_tree)6646# check for various structs that are normally const (ops, kgdb, device_tree)
6645# and avoid what seem like struct definitions 'struct foo {'6647# and avoid what seem like struct definitions 'struct foo {'
6646 if ($line !~ /\bconst\b/ &&6648 if (defined($const_structs) &&
6649 $line !~ /\bconst\b/ &&
6647 $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {6650 $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {
6648 WARN("CONST_STRUCT",6651 WARN("CONST_STRUCT",
6649 "struct $1 should normally be const\n" . $herecurr);6652 "struct $1 should normally be const\n" . $herecurr);