]> git.proxmox.com Git - mirror_qemu.git/blobdiff - scripts/checkpatch.pl
checkpatch: do not warn for multiline parenthesized returned value
[mirror_qemu.git] / scripts / checkpatch.pl
index 25bf43bad0aacd8af6ffc3e5578485e37893ecee..2f81371ffb0fa9b03549e1aeb8e2b14ea37ab1be 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;
@@ -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;
@@ -202,7 +226,6 @@ our $Attribute      = qr{
                        QEMU_NORETURN|
                        QEMU_WARN_UNUSED_RESULT|
                        QEMU_SENTINEL|
-                       QEMU_ARTIFICIAL|
                        QEMU_PACKED|
                        GCC_FMT_ATTR
                  }x;
@@ -242,6 +265,7 @@ our $UTF8   = qr{
 # There are still some false positives, but this catches most
 # common cases.
 our $typeTypedefs = qr{(?x:
+        (?![KMGTPE]iB)                      # IEC binary prefix (do not match)
         [A-Z][A-Z\d_]*[a-z][A-Za-z\d_]*     # camelcase
         | [A-Z][A-Z\d_]*AIOCB               # all uppercase
         | [A-Z][A-Z\d_]*CPU                 # all uppercase
@@ -269,8 +293,36 @@ our @typeList = (
        qr{${Ident}_handler_fn},
        qr{target_(?:u)?long},
        qr{hwaddr},
+        # external libraries
        qr{xml${Ident}},
-       qr{xendevicemodel_handle},
+       qr{xen\w+_handle},
+       # Glib definitions
+       qr{gchar},
+       qr{gshort},
+       qr{glong},
+       qr{gint},
+       qr{gboolean},
+       qr{guchar},
+       qr{gushort},
+       qr{gulong},
+       qr{guint},
+       qr{gfloat},
+       qr{gdouble},
+       qr{gpointer},
+       qr{gconstpointer},
+       qr{gint8},
+       qr{guint8},
+       qr{gint16},
+       qr{guint16},
+       qr{gint32},
+       qr{guint32},
+       qr{gint64},
+       qr{guint64},
+       qr{gsize},
+       qr{gssize},
+       qr{goffset},
+       qr{gintptr},
+       qr{guintptr},
 );
 
 # This can be modified by sub possible.  Since it can be empty, be careful
@@ -311,13 +363,18 @@ 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;
@@ -325,21 +382,33 @@ if ($chk_branch) {
        die "$P: no revisions returned for revlist '$chk_branch'\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) {
@@ -358,6 +427,7 @@ if ($chk_branch) {
                } else {
                        $vname = $filename;
                }
+               print "Checking $filename...\n" if @ARGV > 1 && $quiet == 0;
                while (<$FILE>) {
                        chomp;
                        push(@rawlines, $_);
@@ -1103,11 +1173,10 @@ sub possible {
                        case|
                        else|
                        asm|__asm__|
-                       do|
-                       \#|
-                       \#\#
+                       do
                )(?:\s|$)|
-               ^(?:typedef|struct|enum)\b
+               ^(?:typedef|struct|enum)\b|
+               ^\#
            )}x;
        warn "CHECK<$possible> ($line)\n" if ($dbg_possible > 2);
        if ($possible !~ $notPermitted) {
@@ -1117,7 +1186,7 @@ sub possible {
                if ($possible =~ /^\s*$/) {
 
                } elsif ($possible =~ /\s/) {
-                       $possible =~ s/\s*$Type\s*//g;
+                       $possible =~ s/\s*(?:$Type|\#\#)\s*//g;
                        for my $modifier (split(' ', $possible)) {
                                if ($modifier !~ $notPermitted) {
                                        warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
@@ -1138,14 +1207,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;
 }
@@ -1153,13 +1231,13 @@ 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++;
        }
@@ -1183,9 +1261,9 @@ sub process {
        my $signoff = 0;
        my $is_patch = 0;
 
-       my $in_header_lines = 1;
+       my $in_header_lines = $file ? 0 : 1;
        my $in_commit_log = 0;          #Scanning lines before patch
-
+       my $reported_maintainer_file = 0;
        my $non_utf8_charset = 0;
 
        our @report = ();
@@ -1339,10 +1417,10 @@ sub process {
                # extract the filename as it passes
                if ($line =~ /^diff --git.*?(\S+)$/) {
                        $realfile = $1;
-                       $realfile =~ s@^([^/]*)/@@;
+                       $realfile =~ s@^([^/]*)/@@ if (!$file);
                } elsif ($line =~ /^\+\+\+\s+(\S+)/) {
                        $realfile = $1;
-                       $realfile =~ s@^([^/]*)/@@;
+                       $realfile =~ s@^([^/]*)/@@ if (!$file);
 
                        $p1_prefix = $1;
                        if (!$file && $tree && $p1_prefix ne '' &&
@@ -1374,6 +1452,10 @@ sub process {
                        $is_patch = 1;
                }
 
+               if ($line =~ /^Author: .*via Qemu-devel.*<qemu-devel\@nongnu.org>/) {
+                   ERROR("Author email address is mangled by the mailing list\n" . $herecurr);
+               }
+
 #check the patch for a signoff:
                if ($line =~ /^\s*signed-off-by:/i) {
                        # This is a signoff, if ugly, so do not double report.
@@ -1390,6 +1472,22 @@ 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))))) {
+                       $reported_maintainer_file = 1;
+                       WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
+               }
+
 # Check for wrappage within a valid hunk of the file
                if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) {
                        ERROR("patch seems to be corrupt (line wrapped?)\n" .
@@ -1411,7 +1509,8 @@ sub process {
 # 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 =~ /^$/ &&
-                   $rawline !~ /^(commit\b|from\b|\w+:).+$/i) {
+                   !($rawline =~ /^\s+\S/ ||
+                     $rawline =~ /^(commit\b|from\b|[\w-]+:).*$/i)) {
                        $in_header_lines = 0;
                        $in_commit_log = 1;
                }
@@ -1521,6 +1620,54 @@ sub process {
 # check we are in a valid C source file if not then ignore this hunk
                next if ($realfile !~ /\.(h|c|cpp)$/);
 
+# Block comment styles
+
+               # Block comments use /* on a line of its own
+               if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&      #inline /*...*/
+                   $rawline =~ m@^\+.*/\*\*?+[ \t]*[^ \t]@) { # /* or /** non-blank
+                       WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
+               }
+
+# Block comments use * on subsequent lines
+               if ($prevline =~ /$;[ \t]*$/ &&                 #ends in comment
+                   $prevrawline =~ /^\+.*?\/\*/ &&             #starting /*
+                   $prevrawline !~ /\*\/[ \t]*$/ &&            #no trailing */
+                   $rawline =~ /^\+/ &&                        #line is new
+                   $rawline !~ /^\+[ \t]*\*/) {                #no leading *
+                       WARN("Block comments use * on subsequent lines\n" . $hereprev);
+               }
+
+# Block comments use */ on trailing lines
+               if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ &&       #trailing */
+                   $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&      #inline /*...*/
+                   $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ &&       #trailing **/
+                   $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) {    #non blank */
+                       WARN("Block comments use a trailing */ on a separate line\n" . $herecurr);
+               }
+
+# Block comment * alignment
+               if ($prevline =~ /$;[ \t]*$/ &&                 #ends in comment
+                   $line =~ /^\+[ \t]*$;/ &&                   #leading comment
+                   $rawline =~ /^\+[ \t]*\*/ &&                #leading *
+                   (($prevrawline =~ /^\+.*?\/\*/ &&           #leading /*
+                     $prevrawline !~ /\*\/[ \t]*$/) ||         #no trailing */
+                    $prevrawline =~ /^\+[ \t]*\*/)) {          #leading *
+                       my $oldindent;
+                       $prevrawline =~ m@^\+([ \t]*/?)\*@;
+                       if (defined($1)) {
+                               $oldindent = expand_tabs($1);
+                       } else {
+                               $prevrawline =~ m@^\+(.*/?)\*@;
+                               $oldindent = expand_tabs($1);
+                       }
+                       $rawline =~ m@^\+([ \t]*)\*@;
+                       my $newindent = $1;
+                       $newindent = expand_tabs($newindent);
+                       if (length($oldindent) ne length($newindent)) {
+                               WARN("Block comments should align the * on each line\n" . $hereprev);
+                       }
+               }
+
 # Check for potential 'bare' types
                my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
                    $realline_next);
@@ -1802,7 +1949,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.
@@ -1884,9 +2032,8 @@ sub process {
                        my ($where, $prefix) = ($-[1], $1);
                        if ($prefix !~ /$Type\s+$/ &&
                            ($where != 0 || $prefix !~ /^.\s+$/) &&
-                           $prefix !~ /{\s+$/ &&
                            $prefix !~ /\#\s*define[^(]*\([^)]*\)\s+$/ &&
-                           $prefix !~ /,\s+$/) {
+                           $prefix !~ /[,{:]\s+$/) {
                                ERROR("space prohibited before open square bracket '['\n" . $herecurr);
                        }
                }
@@ -2149,7 +2296,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+/) {
@@ -2164,6 +2312,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);
@@ -2704,7 +2857,8 @@ sub process {
                                info_vreport|
                                error_report|
                                warn_report|
-                               info_report}x;
+                               info_report|
+                               g_test_message}x;
 
        if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
                ERROR("Error messages should not contain newlines\n" . $herecurr);
@@ -2768,30 +2922,31 @@ 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) {
-               exit(0);
+               return 1;
        }
 
        # In mailback mode only produce a report in the negative, for
        # things that appear to be patches.
        if ($mailback && ($clean == 1 || !$is_patch)) {
-               exit(0);
+               return 1;
        }
 
        # This is not a patch, and we are are in 'no-patch' mode so
        # just keep quiet.
        if (!$chk_patch && !$is_patch) {
-               exit(0);
+               return 1;
        }
 
        if (!$is_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)) {