[libvirt] [PATCH 1/2] syntax-check: revert indentation checks
Ján Tomko
jtomko at redhat.com
Fri Oct 5 11:48:06 UTC 2018
Recent patches added indentation checks that discovered some cosmetic
issues at the cost of making this check last as long as the rest of
syntax-check combined on my system. Also, they're moving closer
to us implementing yet another C parser (docs/apibuild.py being the
other one).
Revert the following commits:
commit 11e1f11dd34f2688169c63c13ea8d99a64724369
syntax-check: Check for incorrect indentation in function body
commit 2585a79e32e8b0d994ab35fd7c641eb9b96632e3
build-aux:check-spacing: Introduce a new rule to check misaligned stuff in parenthesises
commit a033182f042a07ffbd4b9a50418670778ceddbf3
build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
commit 6225626b6f0a4817d1f17de0bc5200c5d7986a3e
build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
commit c3875129d9bd094ffe90d54fbec86624ae88c40b
build-aux:check-spacing: Add wrapper function of KillComments
commit e995904c5691be3c21f4c6dbc1f067fe0c8e8515
build-aux:check-spacing: Add wrapper function of CheckFunctionBody
commit 11e1f11dd34f2688169c63c13ea8d99a64724369
syntax-check: Check for incorrect indentation in function body
This brings the speed of the script to a tolerable level and lets it
focus on the more visible issues.
Signed-off-by: Ján Tomko <jtomko at redhat.com>
---
build-aux/check-spacing.pl | 460 ++++++++++++++-------------------------------
1 file changed, 142 insertions(+), 318 deletions(-)
diff --git a/build-aux/check-spacing.pl b/build-aux/check-spacing.pl
index d36b004abf..ca8b434916 100755
--- a/build-aux/check-spacing.pl
+++ b/build-aux/check-spacing.pl
@@ -23,356 +23,180 @@
use strict;
use warnings;
-#
-# CheckFunctionBody:
-# $_[0]: $data(in)
-# $_[1]: $location(in), which format is file-path:line-num:line-code
-# $_[2]: $fn_linenum(inout), maintains start line-num of function body
-# Returns 0 in case of success or 1 on failure
-#
-# Check incorrect indentation and blank first line in function body.
-# For efficiency, it only checks the first line of function body.
-# But it's enough for most cases.
-# (It could be better that we use *state* to declare @fn_linenum and
-# move it into this subroutine. But *state* requires version >= v5.10.)
-#
-sub CheckFunctionBody {
- my $ret = 0;
- my ($data, $location, $fn_linenum) = @_;
+my $ret = 0;
+my $incomment = 0;
- # Check first line of function block
- if ($$fn_linenum) {
- if ($$data =~ /^\s*$/) {
- print "Blank line before content in function body:\n$$location";
- $ret = 1;
- } elsif ($$data !~ /^[ ]{4}\S/) {
- unless ($$data =~ /^[ ]\w+:$/ || $$data =~ /^}/) {
- print "Incorrect indentation in function body:\n$$location";
- $ret = 1;
- }
- }
- $$fn_linenum = 0;
- }
+foreach my $file (@ARGV) {
+ # Per-file variables for multiline Curly Bracket (cb_) check
+ my $cb_linenum = 0;
+ my $cb_code = "";
+ my $cb_scolon = 0;
- # Detect start of function block
- if ($$data =~ /^{$/) {
- $$fn_linenum = $.;
- }
+ open FILE, $file;
- return $ret;
-}
+ while (defined (my $line = <FILE>)) {
+ my $data = $line;
+ # For temporary modifications
+ my $tmpdata;
-#
-# KillComments:
-# $_[0]: $data(inout)
-# $_[1]: $incomment(inout)
-#
-# Remove all content of comments
-# (Also, the @incomment could be declared with *state* and move it in.)
-#
-sub KillComments {
- my ($data, $incomment) = @_;
+ # Kill any quoted , ; = or "
+ $data =~ s/'[";,=]'/'X'/g;
- # Kill contents of multi-line comments
- # and detect end of multi-line comments
- if ($$incomment) {
- if ($$data =~ m,\*/,) {
- $$incomment = 0;
- $$data =~ s,^.*\*/,*/,;
- } else {
- $$data = "";
- }
- }
+ # Kill any quoted strings
+ $data =~ s,"(?:[^\\\"]|\\.)*","XXX",g;
- # Kill single line comments, and detect
- # start of multi-line comments
- if ($$data =~ m,/\*.*\*/,) {
- $$data =~ s,/\*.*\*/,/* */,;
- } elsif ($$data =~ m,/\*,) {
- $$incomment = 1;
- $$data =~ s,/\*.*,/*,;
- }
+ # Kill any C++ style comments
+ $data =~ s,//.*$,//,;
- return;
-}
+ next if $data =~ /^#/;
-#
-# CheckWhiteSpaces:
-# $_[0]: $data(in)
-# $_[1]: $location(in), which format is file-path:line-num:line-code
-# Returns 0 in case of success or 1 on failure
-#
-# Check whitespaces according to code spec of libvirt.
-#
-sub CheckWhiteSpaces {
- my $ret = 0;
- my ($data, $location) = @_;
+ # Kill contents of multi-line comments
+ # and detect end of multi-line comments
+ if ($incomment) {
+ if ($data =~ m,\*/,) {
+ $incomment = 0;
+ $data =~ s,^.*\*/,*/,;
+ } else {
+ $data = "";
+ }
+ }
- # We need to match things like
- #
- # int foo (int bar, bool wizz);
- # foo (bar, wizz);
- #
- # but not match things like:
- #
- # typedef int (*foo)(bar wizz)
- #
- # we can't do this (efficiently) without
- # missing things like
- #
- # foo (*bar, wizz);
- #
- # We also don't want to spoil the $data so it can be used
- # later on.
+ # Kill single line comments, and detect
+ # start of multi-line comments
+ if ($data =~ m,/\*.*\*/,) {
+ $data =~ s,/\*.*\*/,/* */,;
+ } elsif ($data =~ m,/\*,) {
+ $incomment = 1;
+ $data =~ s,/\*.*,/*,;
+ }
- # For temporary modifications
- my $tmpdata = $$data;
- while ($tmpdata =~ /(\w+)\s\((?!\*)/) {
- my $kw = $1;
+ # We need to match things like
+ #
+ # int foo (int bar, bool wizz);
+ # foo (bar, wizz);
+ #
+ # but not match things like:
+ #
+ # typedef int (*foo)(bar wizz)
+ #
+ # we can't do this (efficiently) without
+ # missing things like
+ #
+ # foo (*bar, wizz);
+ #
+ # We also don't want to spoil the $data so it can be used
+ # later on.
+ $tmpdata = $data;
+ while ($tmpdata =~ /(\w+)\s\((?!\*)/) {
+ my $kw = $1;
+
+ # Allow space after keywords only
+ if ($kw =~ /^(?:if|for|while|switch|return)$/) {
+ $tmpdata =~ s/(?:$kw\s\()/XXX(/;
+ } else {
+ print "Whitespace after non-keyword:\n";
+ print "$file:$.: $line";
+ $ret = 1;
+ last;
+ }
+ }
- # Allow space after keywords only
- if ($kw =~ /^(?:if|for|while|switch|return)$/) {
- $tmpdata =~ s/(?:$kw\s\()/XXX(/;
- } else {
- print "Whitespace after non-keyword:\n$$location";
+ # Require whitespace immediately after keywords
+ if ($data =~ /\b(?:if|for|while|switch|return)\(/) {
+ print "No whitespace after keyword:\n";
+ print "$file:$.: $line";
$ret = 1;
- last;
}
- }
- # Require whitespace immediately after keywords
- if ($$data =~ /\b(?:if|for|while|switch|return)\(/) {
- print "No whitespace after keyword:\n$$location";
- $ret = 1;
- }
-
- # Forbid whitespace between )( of a function typedef
- if ($$data =~ /\(\*\w+\)\s+\(/) {
- print "Whitespace between ')' and '(':\n$$location";
- $ret = 1;
- }
-
- # Forbid whitespace following ( or prior to )
- # but allow whitespace before ) on a single line
- # (optionally followed by a semicolon)
- if (($$data =~ /\s\)/ && not $$data =~ /^\s+\);?$/) ||
- $$data =~ /\((?!$)\s/) {
- print "Whitespace after '(' or before ')':\n$$location";
- $ret = 1;
- }
-
- # Forbid whitespace before ";" or ",". Things like below are allowed:
- #
- # 1) The expression is empty for "for" loop. E.g.
- # for (i = 0; ; i++)
- #
- # 2) An empty statement. E.g.
- # while (write(statuswrite, &status, 1) == -1 &&
- # errno == EINTR)
- # ;
- #
- if ($$data =~ /\s[;,]/) {
- unless ($$data =~ /\S; ; / ||
- $$data =~ /^\s+;/) {
- print "Whitespace before semicolon or comma:\n$$location";
+ # Forbid whitespace between )( of a function typedef
+ if ($data =~ /\(\*\w+\)\s+\(/) {
+ print "Whitespace between ')' and '(':\n";
+ print "$file:$.: $line";
$ret = 1;
}
- }
-
- # Require EOL, macro line continuation, or whitespace after ";".
- # Allow "for (;;)" as an exception.
- if ($$data =~ /;[^ \\\n;)]/) {
- print "Invalid character after semicolon:\n$$location";
- $ret = 1;
- }
-
- # Require EOL, space, or enum/struct end after comma.
- if ($$data =~ /,[^ \\\n)}]/) {
- print "Invalid character after comma:\n$$location";
- $ret = 1;
- }
-
- # Require spaces around assignment '=', compounds and '=='
- if ($$data =~ /[^ ]\b[!<>&|\-+*\/%\^=]?=/ ||
- $$data =~ /=[^= \\\n]/) {
- print "Spacing around '=' or '==':\n$$location";
- $ret = 1;
- }
-
- return $ret;
-}
-#
-# CheckCurlyBrackets:
-# $_[0]: $data(in)
-# $_[1]: $file(in)
-# $_[2]: $line(in)
-# $_[3]: $cb_linenum(inout)
-# $_[4]: $cb_code(inout)
-# $_[5]: $cb_scolon(inout)
-# Returns 0 in case of success or 1 on failure
-#
-# Check whitespaces according to code spec of libvirt.
-#
-sub CheckCurlyBrackets {
- my $ret = 0;
- my ($data, $file, $line, $cb_linenum, $cb_code, $cb_scolon) = @_;
-
- # One line conditional statements with one line bodies should
- # not use curly brackets.
- if ($$data =~ /^\s*(if|while|for)\b.*\{$/) {
- $$cb_linenum = $.;
- $$cb_code = $$line;
- $$cb_scolon = 0;
- }
-
- # We need to check for exactly one semicolon inside the body,
- # because empty statements (e.g. with comment only) are
- # allowed
- if ($$cb_linenum == $. - 1 && $$data =~ /^[^;]*;[^;]*$/) {
- $$cb_code .= $$line;
- $$cb_scolon = 1;
- }
-
- if ($$data =~ /^\s*}\s*$/ &&
- $$cb_linenum == $. - 2 &&
- $$cb_scolon) {
-
- print "Curly brackets around single-line body:\n";
- print "$$file:$$cb_linenum-$.:\n$$cb_code$$line";
- $ret = 1;
-
- # There _should_ be no need to reset the values; but to
- # keep my inner peace...
- $$cb_linenum = 0;
- $$cb_scolon = 0;
- $$cb_code = "";
- }
-
- return $ret;
-}
-
-#
-# CheckMisalignment:
-# $_[0]: $data(in)
-# $_[1]: $file(in)
-# $_[2]: $line(in)
-# $_[3]: @paren_stack(inout), which maintains information
-# of the parenthesis
-# Returns 0 in case of success or 1 on failure
-#
-# Check misaligned stuff in parenthesis:
-# 1. For misaligned arguments of function
-# 2. For misaligned conditions of [if|while|switch|...]
-#
-sub CheckMisalignment {
- my $ret = 0;
- my ($data, $file, $line, $paren_stack) = @_;
-
- # Check alignment based on @paren_stack
- if (@$paren_stack) {
- if ($$data =~ /(\S+.*$)/) {
- my $pos = $$paren_stack[-1][0];
- my $linenum = $$paren_stack[-1][1];
- my $code = $$paren_stack[-1][2];
- if ($pos + 1 != length($`)) {
- my $pad = "";
- if ($. > $linenum + 1) {
- $pad = " " x $pos . " ...\n";
- }
- print "Misaligned line in parenthesis:\n";
- print "$$file:$linenum-$.:\n$code$pad$$line\n";
- $ret = 1;
- }
+ # Forbid whitespace following ( or prior to )
+ # but allow whitespace before ) on a single line
+ # (optionally followed by a semicolon)
+ if (($data =~ /\s\)/ && not $data =~ /^\s+\);?$/) ||
+ $data =~ /\((?!$)\s/) {
+ print "Whitespace after '(' or before ')':\n";
+ print "$file:$.: $line";
+ $ret = 1;
}
- }
- # Maintain @paren_stack
- if ($$data =~ /.*[()]/) {
- my $pos = 0;
- my $temp = $$data;
-
- # Kill the content between matched parenthesis and themselves
- # within the current line.
- $temp =~ s,(\((?:[^()]++|(?R))*+\)),"X" x (length $&),ge;
-
- # Pop a item for the open-paren when finding close-paren
- while (($pos = index($temp, "\)", $pos)) >= 0) {
- if (@$paren_stack) {
- pop(@$paren_stack);
- $pos++;
- } else {
- print "Warning: found unbalanced parenthesis:\n";
- print "$$file:$.:\n$$line\n";
+ # Forbid whitespace before ";" or ",". Things like below are allowed:
+ #
+ # 1) The expression is empty for "for" loop. E.g.
+ # for (i = 0; ; i++)
+ #
+ # 2) An empty statement. E.g.
+ # while (write(statuswrite, &status, 1) == -1 &&
+ # errno == EINTR)
+ # ;
+ #
+ if ($data =~ /\s[;,]/) {
+ unless ($data =~ /\S; ; / ||
+ $data =~ /^\s+;/) {
+ print "Whitespace before semicolon or comma:\n";
+ print "$file:$.: $line";
$ret = 1;
- last;
}
}
- # Push the item for open-paren on @paren_stack
- # @item = [ position of the open-paren, linenum, code-line ]
- while (($pos = index($temp, "\(", $pos)) >= 0) {
- push @$paren_stack, [$pos, $., $$line];
- $pos++;
+ # Require EOL, macro line continuation, or whitespace after ";".
+ # Allow "for (;;)" as an exception.
+ if ($data =~ /;[^ \\\n;)]/) {
+ print "Invalid character after semicolon:\n";
+ print "$file:$.: $line";
+ $ret = 1;
}
- }
- return $ret;
-}
-
-my $ret = 0;
-
-foreach my $file (@ARGV) {
- # Per-file variables for multiline Curly Bracket (cb_) check
- my $cb_linenum = 0;
- my $cb_code = "";
- my $cb_scolon = 0;
- my $fn_linenum = 0;
- my $incomment = 0;
- my @paren_stack;
-
- open FILE, $file;
-
- while (defined (my $line = <FILE>)) {
- my $has_define = 0;
- my $data = $line;
- my $location = "$file:$.:\n$line";
-
- # Kill any quoted , ; = or "
- $data =~ s/'[";,=]'/'X'/g;
-
- # Kill any quoted strings. Replace with equal-length "XXXX..."
- $data =~ s,"(([^\\\"]|\\.)*)","\"".'X'x(length $1)."\"",ge;
- $data =~ s,'(([^\\\']|\\.)*)',"\'".'X'x(length $1)."\'",ge;
+ # Require EOL, space, or enum/struct end after comma.
+ if ($data =~ /,[^ \\\n)}]/) {
+ print "Invalid character after comma:\n";
+ print "$file:$.: $line";
+ $ret = 1;
+ }
- # Kill any C++ style comments
- $data =~ s,//.*$,//,;
+ # Require spaces around assignment '=', compounds and '=='
+ if ($data =~ /[^ ]\b[!<>&|\-+*\/%\^=]?=/ ||
+ $data =~ /=[^= \\\n]/) {
+ print "Spacing around '=' or '==':\n";
+ print "$file:$.: $line";
+ $ret = 1;
+ }
- $has_define = 1 if $data =~ /(?:^#\s*define\b)/;
- if (not $has_define) {
- # Ignore all macros except for #define
- next if $data =~ /^#/;
+ # One line conditional statements with one line bodies should
+ # not use curly brackets.
+ if ($data =~ /^\s*(if|while|for)\b.*\{$/) {
+ $cb_linenum = $.;
+ $cb_code = $line;
+ $cb_scolon = 0;
+ }
- $ret = 1 if CheckFunctionBody(\$data, \$location, \$fn_linenum);
+ # We need to check for exactly one semicolon inside the body,
+ # because empty statements (e.g. with comment only) are
+ # allowed
+ if ($cb_linenum == $. - 1 && $data =~ /^[^;]*;[^;]*$/) {
+ $cb_code .= $line;
+ $cb_scolon = 1;
+ }
- KillComments(\$data, \$incomment);
+ if ($data =~ /^\s*}\s*$/ &&
+ $cb_linenum == $. - 2 &&
+ $cb_scolon) {
- $ret = 1 if CheckWhiteSpaces(\$data, \$location);
+ print "Curly brackets around single-line body:\n";
+ print "$file:$cb_linenum-$.:\n$cb_code$line";
+ $ret = 1;
- $ret = 1 if CheckCurlyBrackets(\$data, \$file, \$line,
- \$cb_linenum, \$cb_code, \$cb_scolon);
+ # There _should_ be no need to reset the values; but to
+ # keep my inner peace...
+ $cb_linenum = 0;
+ $cb_scolon = 0;
+ $cb_code = "";
}
-
- #####################################################################
- # Temporary Filter for CheckMisalignment:
- # Here we introduce a white-list of path, since there're
- # too much misalignment.
- # We _need_ fix these misalignment in batches.
- # We _should_ remove it as soon as fixing all.
- #####################################################################
- next unless $file =~ /^src\/util\//;
-
- $ret = 1 if CheckMisalignment(\$data, \$file, \$line, \@paren_stack);
}
close FILE;
}
--
2.16.4
More information about the libvir-list
mailing list