Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Eric Blake
On 05/25/2013 03:13 PM, Martin Kletzander wrote:
 @@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
  while (defined (my $line = FILE)) {
  my $data = $line;

 +# Kill any quoted ; or 
 +$data =~ s,'[;]','X',g;
 +
 
 Good catch, I didn't know this was missing.

Yep, I hit some false positives where we parse for ';' if I didn't add this.

 
 [...]
 
 ACK,

Thanks; pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Osier Yang

On 25/05/13 01:19, Eric Blake wrote:

Enforce the style cleanup in the previous patch.

* build-aux/bracket-spacing.pl: Enforce trailing spacing.
* cfg.mk (bracket-spacing-check): Tweak error wording.
* docs/hacking.html.in: Document the rule.
* HACKING: Regenerate.

Signed-off-by: Eric Blake ebl...@redhat.com
---
  HACKING  | 23 +++
  build-aux/bracket-spacing.pl | 12 
  cfg.mk   |  2 +-
  docs/hacking.html.in | 29 +
  4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/HACKING b/HACKING
index 2bd6d69..42f76b6 100644
--- a/HACKING
+++ b/HACKING
@@ -318,6 +318,29 @@ immediately prior to any closing bracket. E.g.
int foo(int wizz);// Good


+Semicolons
+==
+Semicolons should never have a space beforehand. Inside the condition of a
+for loop, there should always be a space or line break after each semicolon,
+except for the special case of an infinite loop (although more infinite loops
+use while). While not enforced, loop counters generally use post-increment.
+
+  for (i = 0 ;i  limit ; ++i) { // Bad
+  for (i = 0; i  limit; i++) { // Good
+  for (;;) { // ok
+  while (1) { // Better
+
+Empty loop bodies are better represented with curly braces and a comment,
+although use of a semicolon is not currently rejected.
+
+  while ((rc = waitpid(pid, st, 0) == -1)  // ok
+ errno == EINTR);
+  while ((rc = waitpid(pid, st, 0) == -1)  // Better
+ errno == EINTR) {
+  /* nothing */
+  }
+
+
  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
index 2b4..de40040 100755
--- a/build-aux/bracket-spacing.pl
+++ b/build-aux/bracket-spacing.pl
@@ -1,6 +1,7 @@
  #!/usr/bin/perl
  #
  # bracket-spacing.pl: Report any usage of 'function (..args..)'
+# Also check for other syntax issues, such as correct use of ';'
  #
  # This library is free software; you can redistribute it and/or
  # modify it under the terms of the GNU Lesser General Public
@@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
  while (defined (my $line = FILE)) {
  my $data = $line;

+# Kill any quoted ; or 
+$data =~ s,'[;]','X',g;
+
  # Kill any quoted strings
  $data =~ s,([^\\\]|\\.)*,XXX,g;

@@ -125,6 +129,14 @@ foreach my $file (@ARGV) {
  $ret = 1;
  last;
  }
+
+# Require EOL, macro line continuation, or whitespace after :.
+# Allow for (;;) as an exception.
+while ($data =~ /;[^\\\n;)]/) {
+print $file:$.: $line;
+$ret = 1;
+last;
+}
  }
  close FILE;
  }
diff --git a/cfg.mk b/cfg.mk
index 55359e8..6e8b6d4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -845,7 +845,7 @@ 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)
+  (echo $(ME): incorrect whitespace, see HACKING for rules  exit 1)


I see you changed this when pushing:

-  (echo $(ME): incorrect whitespace around brackets, see 
HACKING for rules  exit 1)

+ { echo $(ME): incorrect whitespace, see HACKING for rules 2; \
+   exit 1; }

But it breaks build. I guess you indented to write 21 there.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-28 Thread Eric Blake
On 05/28/2013 09:28 AM, Osier Yang wrote:
 On 25/05/13 01:19, Eric Blake wrote:
 Enforce the style cleanup in the previous patch.

 * build-aux/bracket-spacing.pl: Enforce trailing spacing.
 * cfg.mk (bracket-spacing-check): Tweak error wording.
 * docs/hacking.html.in: Document the rule.
 * HACKING: Regenerate.



 +  (echo $(ME): incorrect whitespace, see HACKING for rules
  exit 1)

 I see you changed this when pushing:
 
 -  (echo $(ME): incorrect whitespace around brackets, see
 HACKING for rules  exit 1)
 + { echo $(ME): incorrect whitespace, see HACKING for rules
 2; \
 +   exit 1; }
 
 But it breaks build. I guess you indented to write 21 there.

Phooey.  I was trying to copy how we did it elsewhere, and copied wrong
without testing my final edit.  I'll push the fix shortly.  :(

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-25 Thread Martin Kletzander
On 05/24/2013 07:19 PM, Eric Blake wrote:
 Enforce the style cleanup in the previous patch.
 
 * build-aux/bracket-spacing.pl: Enforce trailing spacing.
 * cfg.mk (bracket-spacing-check): Tweak error wording.
 * docs/hacking.html.in: Document the rule.
 * HACKING: Regenerate.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  HACKING  | 23 +++
  build-aux/bracket-spacing.pl | 12 
  cfg.mk   |  2 +-
  docs/hacking.html.in | 29 +
  4 files changed, 65 insertions(+), 1 deletion(-)
 
[...]
 diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
 index 2b4..de40040 100755
 --- a/build-aux/bracket-spacing.pl
 +++ b/build-aux/bracket-spacing.pl
 @@ -1,6 +1,7 @@
  #!/usr/bin/perl
  #
  # bracket-spacing.pl: Report any usage of 'function (..args..)'
 +# Also check for other syntax issues, such as correct use of ';'
  #
  # This library is free software; you can redistribute it and/or
  # modify it under the terms of the GNU Lesser General Public
 @@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
  while (defined (my $line = FILE)) {
  my $data = $line;
 
 +# Kill any quoted ; or 
 +$data =~ s,'[;]','X',g;
 +

Good catch, I didn't know this was missing.

[...]

ACK,

Martin

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon

2013-05-24 Thread Eric Blake
Enforce the style cleanup in the previous patch.

* build-aux/bracket-spacing.pl: Enforce trailing spacing.
* cfg.mk (bracket-spacing-check): Tweak error wording.
* docs/hacking.html.in: Document the rule.
* HACKING: Regenerate.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 HACKING  | 23 +++
 build-aux/bracket-spacing.pl | 12 
 cfg.mk   |  2 +-
 docs/hacking.html.in | 29 +
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/HACKING b/HACKING
index 2bd6d69..42f76b6 100644
--- a/HACKING
+++ b/HACKING
@@ -318,6 +318,29 @@ immediately prior to any closing bracket. E.g.
   int foo(int wizz);// Good


+Semicolons
+==
+Semicolons should never have a space beforehand. Inside the condition of a
+for loop, there should always be a space or line break after each semicolon,
+except for the special case of an infinite loop (although more infinite loops
+use while). While not enforced, loop counters generally use post-increment.
+
+  for (i = 0 ;i  limit ; ++i) { // Bad
+  for (i = 0; i  limit; i++) { // Good
+  for (;;) { // ok
+  while (1) { // Better
+
+Empty loop bodies are better represented with curly braces and a comment,
+although use of a semicolon is not currently rejected.
+
+  while ((rc = waitpid(pid, st, 0) == -1)  // ok
+ errno == EINTR);
+  while ((rc = waitpid(pid, st, 0) == -1)  // Better
+ errno == EINTR) {
+  /* nothing */
+  }
+
+
 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
index 2b4..de40040 100755
--- a/build-aux/bracket-spacing.pl
+++ b/build-aux/bracket-spacing.pl
@@ -1,6 +1,7 @@
 #!/usr/bin/perl
 #
 # bracket-spacing.pl: Report any usage of 'function (..args..)'
+# Also check for other syntax issues, such as correct use of ';'
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -31,6 +32,9 @@ foreach my $file (@ARGV) {
 while (defined (my $line = FILE)) {
 my $data = $line;

+# Kill any quoted ; or 
+$data =~ s,'[;]','X',g;
+
 # Kill any quoted strings
 $data =~ s,([^\\\]|\\.)*,XXX,g;

@@ -125,6 +129,14 @@ foreach my $file (@ARGV) {
 $ret = 1;
 last;
 }
+
+# Require EOL, macro line continuation, or whitespace after :.
+# Allow for (;;) as an exception.
+while ($data =~ /;[^\\\n;)]/) {
+print $file:$.: $line;
+$ret = 1;
+last;
+}
 }
 close FILE;
 }
diff --git a/cfg.mk b/cfg.mk
index 55359e8..6e8b6d4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -845,7 +845,7 @@ 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)
+  (echo $(ME): incorrect whitespace, 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 78959f3..08b8b4c 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -377,6 +377,35 @@
   int foo(int wizz);// Good
 /pre

+h2a name=semicolonSemicolons/a/h2
+
+p
+  Semicolons should never have a space beforehand.  Inside the
+  condition of a codefor/code loop, there should always be a
+  space or line break after each semicolon, except for the special
+  case of an infinite loop (although more infinite loops
+  use codewhile/code).  While not enforced, loop counters
+  generally use post-increment.
+/p
+pre
+  for (i = 0 ;i lt; limit ; ++i) { // Bad
+  for (i = 0; i lt; limit; i++) { // Good
+  for (;;) { // ok
+  while (1) { // Better
+/pre
+p
+  Empty loop bodies are better represented with curly braces and a
+  comment, although use of a semicolon is not currently rejected.
+/p
+pre
+  while ((rc = waitpid(pid, amp;st, 0) == -1) amp;amp; // ok
+ errno == EINTR);
+  while ((rc = waitpid(pid, amp;st, 0) == -1) amp;amp; // Better
+ errno == EINTR) {
+  /* nothing */
+  }
+/pre
+
 h2a name=curly_bracesCurly braces/a/h2

 p
-- 
1.8.1.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list