diff options
Diffstat (limited to 'scripts/checkpatch.pl')
-rwxr-xr-x | scripts/checkpatch.pl | 392 |
1 files changed, 254 insertions, 138 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 365892d..833f20f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -365,6 +365,18 @@ our @typeList = ( qr{guintptr}, ); +# Match text found in common license boilerplate comments: +# for new files the SPDX-License-Identifier line is sufficient. +our @LICENSE_BOILERPLATE = ( + "licensed under the terms of the GNU GPL", + "under the terms of the GNU General Public License", + "under the terms of the GNU Lesser General Public", + "Permission is hereby granted, free of charge", + "GNU GPL, version 2 or later", + "See the COPYING file" +); +our $LICENSE_BOILERPLATE_RE = join("|", @LICENSE_BOILERPLATE); + # Load common spelling mistakes and build regular expression list. my $misspellings; my %spelling_fix; @@ -1330,29 +1342,6 @@ sub WARN { } } -# According to tests/qtest/bios-tables-test.c: do not -# change expected file in the same commit with adding test -sub checkfilename { - my ($name, $acpi_testexpected, $acpi_nontestexpected) = @_; - - # Note: shell script that rebuilds the expected files is in the same - # directory as files themselves. - # Note: allowed diff list can be changed both when changing expected - # files and when changing tests. - if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) { - $$acpi_testexpected = $name; - } elsif ($name !~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) { - $$acpi_nontestexpected = $name; - } - if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) { - ERROR("Do not add expected files together with tests, " . - "follow instructions in " . - "tests/qtest/bios-tables-test.c: both " . - $$acpi_testexpected . " and " . - $$acpi_nontestexpected . " found\n"); - } -} - sub checkspdx { my ($file, $expr) = @_; @@ -1417,6 +1406,118 @@ sub checkspdx { } } +# All three of the methods below take a 'file info' record +# which is a hash ref containing +# +# 'isgit': 1 if an enhanced git diff or 0 for a plain diff +# 'githeader': 1 if still parsing git patch header, 0 otherwise +# 'linestart': line number of start of file diff +# 'lineend': line number of end of file diff +# 'filenew': the new filename +# 'fileold': the old filename (same as 'new filename' except +# for renames in git diffs) +# 'action': one of 'modified' (always) or 'new' or 'deleted' or +# 'renamed' (git diffs only) +# 'mode': file mode for new/deleted files (git diffs only) +# 'similarity': file similarity when renamed (git diffs only) +# 'facts': hash ref for storing any metadata related to checks +# + +# Called at the end of each patch, with the list of +# real filenames that were seen in the patch +sub process_file_list { + my @fileinfos = @_; + + # According to tests/qtest/bios-tables-test.c: do not + # change expected file in the same commit with adding test + my @acpi_testexpected; + my @acpi_nontestexpected; + + foreach my $fileinfo (@fileinfos) { + # Note: shell script that rebuilds the expected files is in + # the same directory as files themselves. + # Note: allowed diff list can be changed both when changing + # expected files and when changing tests. + if ($fileinfo->{filenew} =~ m#^tests/data/acpi/# && + $fileinfo->{filenew} !~ m#^\.sh$#) { + push @acpi_testexpected, $fileinfo->{filenew}; + } elsif ($fileinfo->{filenew} !~ + m#^tests/qtest/bios-tables-test-allowed-diff.h$#) { + push @acpi_nontestexpected, $fileinfo->{filenew}; + } + } + if (int(@acpi_testexpected) > 0 and int(@acpi_nontestexpected) > 0) { + ERROR("Do not add expected files together with tests, " . + "follow instructions in " . + "tests/qtest/bios-tables-test.c. Files\n\n " . + join("\n ", @acpi_testexpected) . + "\n\nand\n\n " . + join("\n ", @acpi_nontestexpected) . + "\n\nfound in the same patch\n"); + } + + my $sawmaintainers = 0; + my @maybemaintainers; + foreach my $fileinfo (@fileinfos) { + if ($fileinfo->{action} ne "modified" && + $fileinfo->{filenew} !~ m#^tests/data/acpi/#) { + push @maybemaintainers, $fileinfo->{filenew}; + } + if ($fileinfo->{filenew} eq "MAINTAINERS") { + $sawmaintainers = 1; + } + } + + # If we don't see a MAINTAINERS update, prod the user to check + if (int(@maybemaintainers) > 0 && !$sawmaintainers) { + WARN("added, moved or deleted file(s):\n\n " . + join("\n ", @maybemaintainers) . + "\n\nDoes MAINTAINERS need updating?\n"); + } +} + +# Called at the start of processing a diff hunk for a file +sub process_start_of_file { + my $fileinfo = shift; + + # Check for incorrect file permissions + if ($fileinfo->{action} eq "new" && ($fileinfo->{mode} & 0111)) { + my $permhere = $fileinfo->{linestart} . "FILE: " . + $fileinfo->{filenew} . "\n"; + if ($fileinfo->{filenew} =~ + /(\bMakefile.*|\.(c|cc|cpp|h|mak|s|S))$/) { + ERROR("do not set execute permissions for source " . + "files\n" . $permhere); + } + } +} + +# Called at the end of processing a diff hunk for a file +sub process_end_of_file { + my $fileinfo = shift; + + if ($fileinfo->{action} eq "new" && + !exists $fileinfo->{facts}->{sawspdx}) { + if ($fileinfo->{filenew} =~ + /(\.(c|h|py|pl|sh|json|inc|rs)|Makefile.*)$/) { + # source code files MUST have SPDX license declared + ERROR("New file '" . $fileinfo->{filenew} . + "' requires 'SPDX-License-Identifier'"); + } else { + # Other files MAY have SPDX license if appropriate + WARN("Does new file '" . $fileinfo->{filenew} . + "' need 'SPDX-License-Identifier'?"); + } + } + if ($fileinfo->{action} eq "new" && + exists $fileinfo->{facts}->{sawboilerplate}) { + ERROR("New file '" . $fileinfo->{filenew} . "' must " . + "not have license boilerplate header text, only " . + "the SPDX-License-Identifier, unless this file was " . + "copied from existing code already having such text."); + } +} + sub process { my $filename = shift; @@ -1437,13 +1538,10 @@ sub process { my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch - my $reported_maintainer_file = 0; my $reported_mixing_imported_file = 0; my $in_imported_file = 0; my $in_no_imported_file = 0; my $non_utf8_charset = 0; - my $expect_spdx = 0; - my $expect_spdx_file; our @report = (); our $cnt_lines = 0; @@ -1455,7 +1553,10 @@ sub process { my $realfile = ''; my $realline = 0; my $realcnt = 0; + my $fileinfo; + my @fileinfolist; my $here = ''; + my $oldhere = ''; my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; @@ -1468,9 +1569,6 @@ sub process { my %suppress_whiletrailers; my %suppress_export; - my $acpi_testexpected; - my $acpi_nontestexpected; - # Pre-scan the patch sanitizing the lines. sanitise_line_reset(); @@ -1593,18 +1691,54 @@ sub process { $prefix = "$filename:$realline: " if ($emacs && $file); $prefix = "$filename:$linenr: " if ($emacs && !$file); + $oldhere = $here; $here = "#$linenr: " if (!$file); $here = "#$realline: " if ($file); # extract the filename as it passes - if ($line =~ /^diff --git.*?(\S+)$/) { - $realfile = $1; - $realfile =~ s@^([^/]*)/@@ if (!$file); - checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected); + if ($line =~ /^diff --git\s+(\S+)\s+(\S+)$/) { + my $fileold = $1; + my $filenew = $2; + + if (defined $fileinfo) { + $fileinfo->{lineend} = $oldhere; + process_end_of_file($fileinfo) + } + $fileold =~ s@^([^/]*)/@@ if (!$file); + $filenew =~ s@^([^/]*)/@@ if (!$file); + $realfile = $filenew; + + $fileinfo = { + "isgit" => 1, + "githeader" => 1, + "linestart" => $here, + "lineend" => 0, + "fileold" => $fileold, + "filenew" => $filenew, + "action" => "modified", + "mode" => 0, + "similarity" => 0, + "facts" => {}, + }; + push @fileinfolist, $fileinfo; + } elsif (defined $fileinfo && $fileinfo->{githeader} && + $line =~ /^(new|deleted) (?:file )?mode\s+([0-7]+)$/) { + $fileinfo->{action} = $1; + $fileinfo->{mode} = oct($2); + } elsif (defined $fileinfo && $fileinfo->{githeader} && + $line =~ /^similarity index (\d+)%/) { + $fileinfo->{similarity} = int($1); + } elsif (defined $fileinfo && $fileinfo->{githeader} && + $line =~ /^rename (from|to) [\w\/\.\-]+\s*$/) { + $fileinfo->{action} = "renamed"; + # For a no-change rename, we'll never have any "+++..." + # lines, so trigger actions now + if ($1 eq "to" && $fileinfo->{similarity} == 100) { + process_start_of_file($fileinfo); + } } elsif ($line =~ /^\+\+\+\s+(\S+)/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@ if (!$file); - checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected); $p1_prefix = $1; if (!$file && $tree && $p1_prefix ne '' && @@ -1612,6 +1746,30 @@ sub process { WARN("patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); } + if (defined $fileinfo && !$fileinfo->{isgit}) { + $fileinfo->{lineend} = $oldhere; + process_end_of_file($fileinfo); + } + + if (!defined $fileinfo || !$fileinfo->{isgit}) { + $fileinfo = { + "isgit" => 0, + "githeader" => 0, + "linestart" => $here, + "lineend" => 0, + "fileold" => $realfile, + "filenew" => $realfile, + "action" => "modified", + "mode" => 0, + "similarity" => 0, + "facts" => {}, + }; + push @fileinfolist, $fileinfo; + } else { + $fileinfo->{githeader} = 0; + } + process_start_of_file($fileinfo); + next; } @@ -1623,14 +1781,6 @@ sub process { $cnt_lines++ if ($realcnt != 0); -# Check for incorrect file permissions - if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { - my $permhere = $here . "FILE: $realfile\n"; - if ($realfile =~ /(\bMakefile(?:\.objs)?|\.c|\.cc|\.cpp|\.h|\.mak|\.[sS])$/) { - ERROR("do not set execute permissions for source files\n" . $permhere); - } - } - # Only allow Python 3 interpreter if ($realline == 1 && $line =~ /^\+#!\ *\/usr\/bin\/(?:env )?python$/) { @@ -1662,68 +1812,27 @@ sub process { } } -# Check if MAINTAINERS is being updated. If so, there's probably no need to -# emit the "does MAINTAINERS need updating?" message on file add/move/delete - if ($line =~ /^\s*MAINTAINERS\s*\|/) { - $reported_maintainer_file = 1; - } - -# Check for added, moved or deleted files - if (!$reported_maintainer_file && !$in_commit_log && - ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ || - $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ || - ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ && - (defined($1) || defined($2)))) && - !(($realfile ne '') && - defined($acpi_testexpected) && - ($realfile eq $acpi_testexpected))) { - $reported_maintainer_file = 1; - WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr); - } - -# All new files should have a SPDX-License-Identifier tag - if ($line =~ /^new file mode\s*\d+\s*$/) { - if ($expect_spdx) { - if ($expect_spdx_file =~ - /\.(c|h|py|pl|sh|json|inc|Makefile)$/) { - # source code files MUST have SPDX license declared - ERROR("New file '$expect_spdx_file' requires " . - "'SPDX-License-Identifier'"); - } else { - # Other files MAY have SPDX license if appropriate - WARN("Does new file '$expect_spdx_file' need " . - "'SPDX-License-Identifier'?"); - } - } - $expect_spdx = 1; - $expect_spdx_file = undef; - } elsif ($expect_spdx) { - $expect_spdx_file = $realfile unless - defined $expect_spdx_file; - - # SPDX tags may occurr in comments which were - # stripped from '$line', so use '$rawline' - if ($rawline =~ /SPDX-License-Identifier/) { - $expect_spdx = 0; - $expect_spdx_file = undef; - } - } - # Check SPDX-License-Identifier references a permitted license if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) { - &checkspdx($realfile, $1); + $fileinfo->{facts}->{sawspdx} = 1; + &checkspdx($realfile, $1); + } + + if ($rawline =~ /$LICENSE_BOILERPLATE_RE/) { + $fileinfo->{facts}->{sawboilerplate} = 1; } if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) { - my $tag = $1; - my @permitted = qw( - SPDX-License-Identifier - ); - - unless (grep { /^$tag$/ } @permitted) { - ERROR("Tag $tag not permitted in QEMU code, valid " . - "choices are: " . join(", ", @permitted)); - } + my $tag = $1; + my @permitted = qw( + SPDX-License-Identifier + ); + + unless (grep { /^$tag$/ } @permitted) { + ERROR("Tag $tag not permitted in QEMU code, " . + "valid choices are: " . + join(", ", @permitted)); + } } # Check for wrappage within a valid hunk of the file @@ -2304,7 +2413,7 @@ sub process { # missing space after union, struct or enum definition if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?(?:\s+$Ident)?[=\{]/) { - ERROR("missing space after $1 definition\n" . $herecurr); + ERROR("missing space after $1 definition\n" . $herecurr); } # check for spacing round square brackets; allowed: @@ -2599,7 +2708,7 @@ sub process { if ($line =~ /^.\s*(Q(?:S?LIST|SIMPLEQ|TAILQ)_HEAD)\s*\(\s*[^,]/ && $line !~ /^.typedef/) { - ERROR("named $1 should be typedefed separately\n" . $herecurr); + ERROR("named $1 should be typedefed separately\n" . $herecurr); } # Need a space before open parenthesis after if, while etc @@ -3148,48 +3257,50 @@ sub process { # Qemu error function tests - # Find newlines in error messages - my $qemu_error_funcs = qr{error_setg| - error_setg_errno| - error_setg_win32| - error_setg_file_open| - error_set| - error_prepend| - warn_reportf_err| - error_reportf_err| - error_vreport| - warn_vreport| - info_vreport| - error_report| - warn_report| - info_report| - g_test_message}x; - - if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) { - ERROR("Error messages should not contain newlines\n" . $herecurr); - } + # Find newlines in error messages + my $qemu_error_funcs = qr{error_setg| + error_setg_errno| + error_setg_win32| + error_setg_file_open| + error_set| + error_prepend| + warn_reportf_err| + error_reportf_err| + error_vreport| + warn_vreport| + info_vreport| + error_report| + warn_report| + info_report| + g_test_message}x; + + if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) { + ERROR("Error messages should not contain newlines\n" . $herecurr); + } - # Continue checking for error messages that contains newlines. This - # check handles cases where string literals are spread over multiple lines. - # Example: - # error_report("Error msg line #1" - # "Error msg line #2\n"); - my $quoted_newline_regex = qr{\+\s*\".*\\n.*\"}; - my $continued_str_literal = qr{\+\s*\".*\"}; + # Continue checking for error messages that contains newlines. + # This check handles cases where string literals are spread + # over multiple lines. + # Example: + # error_report("Error msg line #1" + # "Error msg line #2\n"); + my $quoted_newline_regex = qr{\+\s*\".*\\n.*\"}; + my $continued_str_literal = qr{\+\s*\".*\"}; - if ($rawline =~ /$quoted_newline_regex/) { - # Backtrack to first line that does not contain only a quoted literal - # and assume that it is the start of the statement. - my $i = $linenr - 2; + if ($rawline =~ /$quoted_newline_regex/) { + # Backtrack to first line that does not contain only + # a quoted literal and assume that it is the start + # of the statement. + my $i = $linenr - 2; - while (($i >= 0) & $rawlines[$i] =~ /$continued_str_literal/) { - $i--; - } + while (($i >= 0) & $rawlines[$i] =~ /$continued_str_literal/) { + $i--; + } - if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) { - ERROR("Error messages should not contain newlines\n" . $herecurr); + if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) { + ERROR("Error messages should not contain newlines\n" . $herecurr); + } } - } # check for non-portable libc calls that have portable alternatives in QEMU if ($line =~ /\bffs\(/) { @@ -3243,6 +3354,11 @@ sub process { } } + if (defined $fileinfo) { + process_end_of_file($fileinfo); + } + process_file_list(@fileinfolist); + if ($is_patch && $chk_signoff && $signoff == 0) { ERROR("Missing Signed-off-by: line(s)\n"); } |