]> 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 06ec14e7f79ef1cf3b01b4dbb4fc33c684a63c11..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;
@@ -340,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;
@@ -354,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) {
@@ -387,6 +427,7 @@ if ($chk_branch) {
                } else {
                        $vname = $filename;
                }
+               print "Checking $filename...\n" if @ARGV > 1 && $quiet == 0;
                while (<$FILE>) {
                        chomp;
                        push(@rawlines, $_);
@@ -1166,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";
 
-       push(our @report, $line);
+       $output = (split('\n', $output))[0] . "\n" if ($terse);
+
+       push(our @report, $output);
 
        return 1;
 }
@@ -1181,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++;
        }
@@ -1570,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);
@@ -1851,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.
@@ -2197,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+/) {
@@ -2212,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);
@@ -2752,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);
@@ -2816,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)) {