Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon
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
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
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
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
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