Document bracket whitespace rules & add syntax-check rule

This documents the following whitespace rules

      if(foo)   // Bad
      if (foo)  // Good

      int foo (int wizz)  // Bad
      int foo(int wizz)   // Good

      bar = foo (wizz);  // Bad
      bar = foo(wizz);   // Good

      typedef int (*foo) (int wizz);  // Bad
      typedef int (*foo)(int wizz);   // Good

      int foo( int wizz );  // Bad
      int foo(int wizz);    // Good

There is a syntax-check rule extension to validate all these rules.
Checking for 'function (...args...)' is quite difficult since it
needs to ignore valid usage with keywords like 'if (...test...)'
and while/for/switch. It must also ignore source comments and
quoted strings.

It is not possible todo this with a simple regex in the normal
syntax-check style. So a short Perl script is created instead
to analyse the source. In practice this works well enough. The
only thing it can't cope with is multi-line quoted strings of
the form

 "start of string\
more lines\
more line\
the end"

but this can and should be written as

 "start of string"
 "more lines"
 "more line"
 "the end"

with this simple change, the bracket checking script does not
have any false positives across libvirt source, provided it
is only run against .c files. It is not practical to run it
against .h files, since those use whitespace extensively to
get alignment (though this is somewhat inconsistent and could
arguably be fixed).

The only limitation is that it cannot detect a violation where
the first arg starts with a '*', eg

   foo(*wizz);

since this generates too many false positives on function
typedefs which can't be supressed efficiently.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/HACKING b/HACKING
index 90d2a1cb173b20c2c4c1f85bdee0dd35c04e67ca..0ee988a8a8110af2454b56a3e4ec14ca262b361f 100644 (file)
--- a/HACKING
+++ b/HACKING
@@ -179,6 +179,37 @@ otherwise. For example, it is preferable to use "/* */" comments rather than
 declare them at the beginning of a scope, rather than immediately before use.
 
 
+Bracket spacing
+===============
+The keywords "if", "for", "while", and "switch" must have a single space
+following them before the opening bracket. eg
+
+      if(foo)   // Bad
+      if (foo)  // Good
+
+Function implementations mustnothave any whitespace between the function name and the opening bracket. eg
+
+      int foo (int wizz)  // Bad
+      int foo(int wizz)   // Good
+
+Function calls mustnothave any whitespace between the function name and the opening bracket. eg
+
+      bar = foo (wizz);  // Bad
+      bar = foo(wizz);   // Good
+
+Function typedefs mustnothave any whitespace between the closing bracket of the function name and
+opening bracket of the arg list. eg
+
+      typedef int (*foo) (int wizz);  // Bad
+      typedef int (*foo)(int wizz);   // Good
+
+There must not be any whitespace immediately following any opening bracket, or
+immediately prior to any closing bracket
+
+      int foo( int wizz );  // Bad
+      int foo(int wizz);    // Good
+
+
 Curly braces
 ============
 Omit the curly braces around an "if", "while", "for" etc. body only when that

diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
new file mode 100755 (executable)
index 0000000..d3a916f
--- /dev/null
+++ b/build-aux/bracket-spacing.pl
@@ -0,0 +1,116 @@
+#!/usr/bin/perl
+#
+# bracket-spacing.pl: Report any usage of 'function (..args..)'
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see
+# <http://www.gnu.org/licenses/>.
+#
+# Authors:
+#     Daniel P. Berrange <berrange@redhat.com>
+
+use strict;
+use warnings;
+
+my $ret = 0;
+my $incomment = 0;
+
+foreach my $file (@ARGV) {
+    open FILE, $file;
+
+    while (defined (my $line = <FILE>)) {
+        my $data = $line;
+
+        # Kill any quoted strongs
+        $data =~ s,".*?","XXX",g;
+
+        # Kill any C++ style comments
+        $data =~ s,//.*$,//,;
+
+        next if $data =~ /^#/;
+
+        # 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 single line comments, and detect
+        # start of multi-line comments
+        if ($data =~ m,/\*.*\*/,) {
+            $data =~ s,/\*.*\*/,/* */,;
+        } elsif ($data =~ m,/\*,) {
+            $incomment = 1;
+            $data =~ s,/\*.*,/*,;
+        }
+
+        # 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);
+        #
+        while ($data =~ /(\w+)\s\((?!\*)/) {
+            my $kw = $1;
+
+            # Allow space after keywords only
+            if ($kw =~ /^(if|for|while|switch|return)$/) {
+                $data =~ s/($kw\s\()/XXX(/;
+            } else {
+                print "$file:$.: $line";
+                $ret = 1;
+                last;
+            }
+        }
+
+        # Require whitespace immediately after keywords,
+        # but none after the opening bracket
+        while ($data =~ /(if|for|while|switch|return)\(/ ||
+               $data =~ /(if|for|while|switch|return)\s+\(\s/) {
+            print "$file:$.: $line";
+            $ret = 1;
+            last;
+        }
+
+        # Forbid whitespace between )( of a function typedef
+        while ($data =~ /\(\*\w+\)\s+\(/) {
+            print "$file:$.: $line";
+            $ret = 1;
+            last;
+        }
+
+        # Forbid whitespace following ( or prior to )
+        while ($data =~ /\S\s+\)/ ||
+               $data =~ /\(\s+\S/) {
+            print "$file:$.: $line";
+            $ret = 1;
+            last;
+        }
+    }
+    close FILE;
+}
+
+exit $ret;

diff --git a/cfg.mk b/cfg.mk
index cda04e418aee5591e62ec7a287ea0b6aa40e1afc..802e94efed719d6e4258133b4c4eef49eccf058f 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -718,7 +718,12 @@ _autogen:
        ./config.status
 
 # regenerate HACKING as part of the syntax-check
-syntax-check: $(top_srcdir)/HACKING
+syntax-check: $(top_srcdir)/HACKING bracket-spacing-check
+
+bracket-spacing-check:
+       $(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \
+       $(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \
+          (echo $(ME): incorrect whitespace around brackets, see HACKING for rules && exit 1)
 
 # sc_po_check can fail if generated files are not built first
 sc_po_check: \

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index d41b39c490d4ac6bce8a91f823b17905fa26defd..40acdbbd9593145a0c2b8375edcb3679a551f249 100644 (file)
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -212,6 +212,55 @@
     </p>
 
 
+    <h2><a name="bracket_spacing">Bracket spacing</a></h2>
+
+    <p>
+      The keywords <code>if</code>, <code>for</code>, <code>while</code>,
+      and <code>switch</code> must have a single space following them
+      before the opening bracket. eg
+    </p>
+    <pre>
+      if(foo)   // Bad
+      if (foo)  // Good
+</pre>
+
+    <p>
+      Function implementations must <strong>not</strong> have any whitespace
+      between the function name and the opening bracket. eg
+    </p>
+    <pre>
+      int foo (int wizz)  // Bad
+      int foo(int wizz)   // Good
+</pre>
+
+    <p>
+      Function calls must <strong>not</strong> have any whitespace
+      between the function name and the opening bracket. eg
+    </p>
+    <pre>
+      bar = foo (wizz);  // Bad
+      bar = foo(wizz);   // Good
+</pre>
+
+    <p>
+      Function typedefs must <strong>not</strong> have any whitespace
+      between the closing bracket of the function name and opening
+      bracket of the arg list. eg
+    </p>
+    <pre>
+      typedef int (*foo) (int wizz);  // Bad
+      typedef int (*foo)(int wizz);   // Good
+</pre>
+
+    <p>
+      There must not be any whitespace immediately following any
+      opening bracket, or immediately prior to any closing bracket
+    </p>
+    <pre>
+      int foo( int wizz );  // Bad
+      int foo(int wizz);    // Good
+</pre>
+
     <h2><a name="curly_braces">Curly braces</a></h2>
 
     <p>