]> git.proxmox.com Git - mirror_qemu.git/blobdiff - scripts/checkpatch.pl
Merge remote-tracking branch 'remotes/berrange-gitlab/tags/misc-next-pull-request...
[mirror_qemu.git] / scripts / checkpatch.pl
index 66eb68e85a0e28f92c9edd5db64bfe097a00f8bb..88c858f67cbb13de1c87adafb08a4f02d2d2f92f 100755 (executable)
@@ -7,6 +7,7 @@
 
 use strict;
 use warnings;
+use Term::ANSIColor qw(:constants);
 
 my $P = $0;
 $P =~ s@.*/@@g;
@@ -26,6 +27,7 @@ my $tst_only;
 my $emacs = 0;
 my $terse = 0;
 my $file = undef;
+my $color = "auto";
 my $no_warnings = 0;
 my $summary = 1;
 my $mailback = 0;
@@ -47,7 +49,7 @@ Version: $V
 
 Options:
   -q, --quiet                quiet
-  --no-tree                  run without a kernel tree
+  --no-tree                  run without a qemu tree
   --no-signoff               do not check for 'Signed-off-by' line
   --patch                    treat FILE as patchfile
   --branch                   treat args as GIT revision list
@@ -55,7 +57,7 @@ Options:
   --terse                    one line per report
   -f, --file                 treat FILE as regular source file
   --strict                   fail if only warnings are found
-  --root=PATH                PATH to the kernel tree root
+  --root=PATH                PATH to the qemu tree root
   --no-summary               suppress the per-file summary
   --mailback                 only produce a report in case of warnings/errors
   --summary-file             include the filename in summary
@@ -64,6 +66,8 @@ Options:
                              is all off)
   --test-only=WORD           report only warnings/errors containing WORD
                              literally
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.
@@ -72,6 +76,14 @@ EOM
        exit($exitcode);
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+       if ($_ eq "--color" || $_ eq "-color") {
+               $_ = "--color=$color";
+       }
+}
+
 GetOptions(
        'q|quiet+'      => \$quiet,
        'tree!'         => \$tree,
@@ -89,6 +101,8 @@ GetOptions(
 
        'debug=s'       => \%debug,
        'test-only=s'   => \$tst_only,
+       'color=s'       => \$color,
+       'no-color'      => sub { $color = 'never'; },
        'h|help'        => \$help,
        'version'       => \$help
 ) or help(1);
@@ -144,6 +158,16 @@ if (!$chk_patch && !$chk_branch && !$file) {
        die "One of --file, --branch, --patch is required\n";
 }
 
+if ($color =~ /^always$/i) {
+       $color = 1;
+} elsif ($color =~ /^never$/i) {
+       $color = 0;
+} elsif ($color =~ /^auto$/i) {
+       $color = (-t STDOUT);
+} else {
+       die "Invalid color mode: $color\n";
+}
+
 my $dbg_values = 0;
 my $dbg_possible = 0;
 my $dbg_type = 0;
@@ -179,7 +203,7 @@ if ($tree) {
        }
 
        if (!defined $root) {
-               print "Must be run from the top-level dir. of a kernel tree\n";
+               print "Must be run from the top-level dir. of a qemu tree\n";
                exit(2);
        }
 }
@@ -238,6 +262,19 @@ our $UTF8  = qr{
        | $NON_ASCII_UTF8
 }x;
 
+# some readers default to ISO-8859-1 when showing email source. detect
+# when UTF-8 is incorrectly interpreted as ISO-8859-1 and reencoded back.
+# False positives are possible but very unlikely.
+our $UTF8_MOJIBAKE = qr{
+       \xC3[\x82-\x9F] \xC2[\x80-\xBF]                    # c2-df 80-bf
+       | \xC3\xA0 \xC2[\xA0-\xBF] \xC2[\x80-\xBF]         # e0 a0-bf 80-bf
+       | \xC3[\xA1-\xAC\xAE\xAF] (?: \xC2[\x80-\xBF]){2}  # e1-ec/ee/ef 80-bf 80-bf
+       | \xC3\xAD \xC2[\x80-\x9F] \xC2[\x80-\xBF]         # ed 80-9f 80-bf
+       | \xC3\xB0 \xC2[\x90-\xBF] (?: \xC2[\x80-\xBF]){2} # f0 90-bf 80-bf 80-bf
+       | \xC3[\xB1-\xB3] (?: \xC2[\x80-\xBF]){3}          # f1-f3 80-bf 80-bf 80-bf
+       | \xC3\xB4 \xC2[\x80-\x8F] (?: \xC2[\x80-\xBF]){2} # f4 80-b8 80-bf 80-bf
+}x;
+
 # There are still some false positives, but this catches most
 # common cases.
 our $typeTypedefs = qr{(?x:
@@ -339,35 +376,52 @@ my @lines = ();
 my $vname;
 if ($chk_branch) {
        my @patches;
+       my %git_commits = ();
        my $HASH;
-       open($HASH, "-|", "git", "log", "--format=%H", $ARGV[0]) ||
-               die "$P: git log --format=%H $ARGV[0] failed - $!\n";
-
-       while (<$HASH>) {
-               chomp;
-               push @patches, $_;
+       open($HASH, "-|", "git", "log", "--reverse", "--no-merges", "--format=%H %s", $ARGV[0]) ||
+               die "$P: git log --reverse --no-merges --format='%H %s' $ARGV[0] failed - $!\n";
+
+       for my $line (<$HASH>) {
+               $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/;
+               next if (!defined($1) || !defined($2));
+               my $sha1 = $1;
+               my $subject = $2;
+               push(@patches, $sha1);
+               $git_commits{$sha1} = $subject;
        }
 
        close $HASH;
 
-       die "$P: no revisions returned for revlist '$chk_branch'\n"
+       die "$P: no revisions returned for revlist '$ARGV[0]'\n"
            unless @patches;
 
+       my $i = 1;
+       my $num_patches = @patches;
        for my $hash (@patches) {
                my $FILE;
                open($FILE, '-|', "git", "show", $hash) ||
                        die "$P: git show $hash - $!\n";
-               $vname = $hash;
                while (<$FILE>) {
                        chomp;
                        push(@rawlines, $_);
                }
                close($FILE);
+               $vname = substr($hash, 0, 12) . ' (' . $git_commits{$hash} . ')';
+               if ($num_patches > 1 && $quiet == 0) {
+                       my $prefix = "$i/$num_patches";
+                       $prefix = BLUE . BOLD . $prefix . RESET if $color;
+                       print "$prefix Checking commit $vname\n";
+                       $vname = "Patch $i/$num_patches";
+               } else {
+                       $vname = "Commit " . $vname;
+               }
                if (!process($hash)) {
                        $exit = 1;
+                       print "\n" if ($num_patches > 1 && $quiet == 0);
                }
                @rawlines = ();
                @lines = ();
+               $i++;
        }
 } else {
        for my $filename (@ARGV) {
@@ -386,6 +440,7 @@ if ($chk_branch) {
                } else {
                        $vname = $filename;
                }
+               print "Checking $filename...\n" if @ARGV > 1 && $quiet == 0;
                while (<$FILE>) {
                        chomp;
                        push(@rawlines, $_);
@@ -406,8 +461,8 @@ sub top_of_kernel_tree {
 
        my @tree_check = (
                "COPYING", "MAINTAINERS", "Makefile",
-               "README", "docs", "VERSION",
-               "vl.c"
+               "README.rst", "docs", "VERSION",
+               "linux-user", "softmmu"
        );
 
        foreach my $check (@tree_check) {
@@ -1165,14 +1220,23 @@ sub possible {
 my $prefix = '';
 
 sub report {
-       if (defined $tst_only && $_[0] !~ /\Q$tst_only\E/) {
+       my ($level, $msg) = @_;
+       if (defined $tst_only && $msg !~ /\Q$tst_only\E/) {
                return 0;
        }
-       my $line = $prefix . $_[0];
 
-       $line = (split('\n', $line))[0] . "\n" if ($terse);
+       my $output = '';
+       $output .= BOLD if $color;
+       $output .= $prefix;
+       $output .= RED if $color && $level eq 'ERROR';
+       $output .= MAGENTA if $color && $level eq 'WARNING';
+       $output .= $level . ':';
+       $output .= RESET if $color;
+       $output .= ' ' . $msg . "\n";
+
+       $output = (split('\n', $output))[0] . "\n" if ($terse);
 
-       push(our @report, $line);
+       push(our @report, $output);
 
        return 1;
 }
@@ -1180,18 +1244,41 @@ sub report_dump {
        our @report;
 }
 sub ERROR {
-       if (report("ERROR: $_[0]\n")) {
+       if (report("ERROR", $_[0])) {
                our $clean = 0;
                our $cnt_error++;
        }
 }
 sub WARN {
-       if (report("WARNING: $_[0]\n")) {
+       if (report("WARNING", $_[0])) {
                our $clean = 0;
                our $cnt_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 process {
        my $filename = shift;
 
@@ -1238,6 +1325,9 @@ sub process {
        my %suppress_whiletrailers;
        my %suppress_export;
 
+        my $acpi_testexpected;
+        my $acpi_nontestexpected;
+
        # Pre-scan the patch sanitizing the lines.
 
        sanitise_line_reset();
@@ -1367,9 +1457,11 @@ sub process {
                if ($line =~ /^diff --git.*?(\S+)$/) {
                        $realfile = $1;
                        $realfile =~ s@^([^/]*)/@@ if (!$file);
+                       checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
                } 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 '' &&
@@ -1396,6 +1488,12 @@ sub process {
                        }
                }
 
+# Only allow Python 3 interpreter
+               if ($realline == 1 &&
+                       $line =~ /^\+#!\ *\/usr\/bin\/(?:env )?python$/) {
+                       ERROR("please use python3 interpreter\n" . $herecurr);
+               }
+
 # Accept git diff extended headers as valid patches
                if ($line =~ /^(?:rename|copy) (?:from|to) [\w\/\.\-]+\s*$/) {
                        $is_patch = 1;
@@ -1455,6 +1553,9 @@ sub process {
                        ERROR("Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr);
                }
 
+               if ($rawline =~ m/$UTF8_MOJIBAKE/) {
+                       ERROR("Doubly-encoded UTF-8\n" . $herecurr);
+               }
 # Check if it's the start of a commit log
 # (not a header line and we haven't seen the patch filename)
                if ($in_header_lines && $realfile =~ /^$/ &&
@@ -1558,7 +1659,7 @@ sub process {
 # tabs are only allowed in assembly source code, and in
 # some scripts we imported from other projects.
                next if ($realfile =~ /\.(s|S)$/);
-               next if ($realfile =~ /(checkpatch|get_maintainer|texi2pod)\.pl$/);
+               next if ($realfile =~ /(checkpatch|get_maintainer)\.pl$/);
 
                if ($rawline =~ /^\+.*\t/) {
                        my $herevet = "$here\n" . cat_vet($rawline) . "\n";
@@ -1572,8 +1673,8 @@ sub process {
 # Block comment styles
 
                # Block comments use /* on a line of its own
-               if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&      #inline /*...*/
-                   $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
+               if ($rawline !~ m@^\+.*/\*.*\*/[ \t)}]*$@ &&    #inline /*...*/
+                   $rawline =~ m@^\+.*/\*\*?+[ \t]*[^ \t]@) { # /* or /** non-blank
                        WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
                }
 
@@ -1757,6 +1858,11 @@ sub process {
                        ERROR("suspicious ; after while (0)\n" . $herecurr);
                }
 
+# Check superfluous trailing ';'
+               if ($line =~ /;;$/) {
+                       ERROR("superfluous trailing semicolon\n" . $herecurr);
+               }
+
 # Check relative indent for conditionals and blocks.
                if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
                        my ($s, $c) = ($stat, $cond);
@@ -1764,7 +1870,7 @@ sub process {
                        substr($s, 0, length($c), '');
 
                        # Make sure we remove the line prefixes as we have
-                       # none on the first line, and are going to readd them
+                       # none on the first line, and are going to re-add them
                        # where necessary.
                        $s =~ s/\n./\n/gs;
 
@@ -1898,7 +2004,8 @@ sub process {
                }
 
 # no C99 // comments
-               if ($line =~ m{//}) {
+               if ($line =~ m{//} &&
+                   $rawline !~ m{// SPDX-License-Identifier: }) {
                        ERROR("do not use C99 // comments\n" . $herecurr);
                }
                # Remove C99 comments.
@@ -2244,7 +2351,8 @@ sub process {
                               $value =~ s/\([^\(\)]*\)/1/) {
                        }
 #print "value<$value>\n";
-                       if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) {
+                       if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/ &&
+                           $line =~ /;$/) {
                                ERROR("return is not a function, parentheses are not required\n" . $herecurr);
 
                        } elsif ($spacing !~ /\s+/) {
@@ -2259,6 +2367,11 @@ sub process {
                        }
                }
 
+               if ($line =~ /^.\s*(Q(?:S?LIST|SIMPLEQ|TAILQ)_HEAD)\s*\(\s*[^,]/ &&
+                   $line !~ /^.typedef/) {
+                   ERROR("named $1 should be typedefed separately\n" . $herecurr);
+               }
+
 # Need a space before open parenthesis after if, while etc
                if ($line=~/\b(if|while|for|switch)\(/) {
                        ERROR("space required before the open parenthesis '('\n" . $herecurr);
@@ -2767,14 +2880,20 @@ sub process {
                                $herecurr);
                }
 
-# check for %L{u,d,i} in strings
+# format strings checks
                my $string;
                while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
                        $string = substr($rawline, $-[1], $+[1] - $-[1]);
                        $string =~ s/%%/__/g;
+                       # check for %L{u,d,i} in strings
                        if ($string =~ /(?<!%)%L[udi]/) {
                                ERROR("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr);
-                               last;
+                       }
+                       # check for %# or %0# in printf-style format strings
+                       if ($string =~ /(?<!%)%0?#/) {
+                               ERROR("Don't use '#' flag of printf format " .
+                                     "('%#') in format strings, use '0x' " .
+                                     "prefix instead\n" . $herecurr);
                        }
                }
 
@@ -2841,6 +2960,12 @@ sub process {
                if ($line =~ /\bbzero\(/) {
                        ERROR("use memset() instead of bzero()\n" . $herecurr);
                }
+               if ($line =~ /\bgetpagesize\(\)/) {
+                       ERROR("use qemu_real_host_page_size instead of getpagesize()\n" . $herecurr);
+               }
+               if ($line =~ /\bsysconf\(_SC_PAGESIZE\)/) {
+                       ERROR("use qemu_real_host_page_size instead of sysconf(_SC_PAGESIZE)\n" . $herecurr);
+               }
                my $non_exit_glib_asserts = qr{g_assert_cmpstr|
                                                g_assert_cmpint|
                                                g_assert_cmpuint|
@@ -2864,6 +2989,10 @@ sub process {
                }
        }
 
+       if ($is_patch && $chk_signoff && $signoff == 0) {
+               ERROR("Missing Signed-off-by: line(s)\n");
+       }
+
        # If we have no input at all, then there is nothing to report on
        # so just keep quiet.
        if ($#rawlines == -1) {
@@ -2882,12 +3011,9 @@ sub process {
                return 1;
        }
 
-       if (!$is_patch) {
+       if (!$is_patch && $filename !~ /cover-letter\.patch$/) {
                ERROR("Does not appear to be a unified-diff format patch\n");
        }
-       if ($is_patch && $chk_signoff && $signoff == 0) {
-               ERROR("Missing Signed-off-by: line(s)\n");
-       }
 
        print report_dump();
        if ($summary && !($clean == 1 && $quiet == 1)) {