Re: [libvirt] [PATCH] syntax-check: check QEMU caps grouping

[John Ferlan]

On 04/12/2018 03:40 AM, Ján Tomko wrote:
> Introduce a perl script that is able to regroup both
> the QEMU_CAPS constants and the capability strings.
> 
> Check correct grouping as a part of syntax check.
> 
> For in-place regrouping after a rebase, just run:
>   tests/group-qemu-caps.pl
> without any parameters.
> 
> Signed-off-by: Ján Tomko 
> ---
>  cfg.mk   |   5 +-
>  src/qemu/qemu_capabilities.h |   2 +-
>  tests/group-qemu-caps.pl | 118 
> +++
>  3 files changed, 123 insertions(+), 2 deletions(-)
>  create mode 100755 tests/group-qemu-caps.pl
> 

Reviewed-by: John Ferlan 

John

Not my area of expertise, but I assume you use this and it looks useful
to reduce code review tasks on capabilities files. One less thing to
think about as long as someone runs syntax-check before posting patches.

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

[libvirt] [PATCH] syntax-check: check QEMU caps grouping

[Ján Tomko]

Introduce a perl script that is able to regroup both
the QEMU_CAPS constants and the capability strings.

Check correct grouping as a part of syntax check.

For in-place regrouping after a rebase, just run:
  tests/group-qemu-caps.pl
without any parameters.

Signed-off-by: Ján Tomko 
---
 cfg.mk   |   5 +-
 src/qemu/qemu_capabilities.h |   2 +-
 tests/group-qemu-caps.pl | 118 +++
 3 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100755 tests/group-qemu-caps.pl

diff --git a/cfg.mk b/cfg.mk
index 4078bc2c6..7782ad6b3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1093,7 +1093,7 @@ _autogen_error:
 
 ifneq ($(_gl-Makefile),)
 syntax-check: spacing-check test-wrap-argv \
-   prohibit-duplicate-header mock-noinline
+   prohibit-duplicate-header mock-noinline group-qemu-caps
 endif
 
 # Don't include duplicate header in the source (either *.c or *.h)
@@ -1114,6 +1114,9 @@ test-wrap-argv:
$(AM_V_GEN)files=`$(VC_LIST) | grep -E '\.(ldargs|args)'`; \
$(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check $$files
 
+group-qemu-caps:
+   $(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check
+
 # sc_po_check can fail if generated files are not built first
 sc_po_check: \
$(srcdir)/src/remote/remote_daemon_dispatch_stubs.h \
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bec28cae9..00c76fdd1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -46,7 +46,7 @@
  * longer be used in code. Periodically we can then purge all the
  * X_ flags and re-group what's left.
  */
-typedef enum {
+typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
 /* 0 */
 X_QEMU_CAPS_KQEMU, /* Whether KQEMU is compiled in */
 X_QEMU_CAPS_VNC_COLON, /* VNC takes or address + display */
diff --git a/tests/group-qemu-caps.pl b/tests/group-qemu-caps.pl
new file mode 100755
index 0..847e36025
--- /dev/null
+++ b/tests/group-qemu-caps.pl
@@ -0,0 +1,118 @@
+#!/usr/bin/env perl
+#
+# 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
+# .
+#
+#
+# Regroup array values into smaller groups separated by numbered comments.
+#
+# If --check is the first parameter, the script will return
+# a non-zero value if a file is not grouped correctly.
+# Otherwise the files are regrouped in place.
+
+use strict;
+use warnings;
+
+my $check = 0;
+
+if (defined $ARGV[0] && $ARGV[0] eq "--check") {
+$check = 1;
+shift @ARGV;
+}
+
+my $ret = 0;
+if (_caps('src/qemu/qemu_capabilities.c',
+  '^VIR_ENUM_IMPL\(virQEMUCaps,',
+  '\);',
+  0,
+  "  ") < 0) {
+$ret = 1;
+}
+if (_caps('src/qemu/qemu_capabilities.h',
+  'virQEMUCapsFlags grouping marker',
+  'QEMU_CAPS_LAST \/\* this must',
+  1,
+  "") < 0) {
+$ret = 1;
+}
+
+exit $ret;
+
+sub regroup_caps {
+my $filename = shift;
+my $start_regex = shift;
+my $end_regex = shift;
+my $trailing_newline = shift;
+my $counter_prefix = shift;
+my $step = 5;
+
+open FILE, '<', $filename or die "cannot open $filename: $!";
+my @original = ;
+close FILE;
+
+my @fixed;
+my $game_on = 0;
+my $counter = 0;
+foreach (@original) {
+if ($game_on) {
+next if ($_ =~ '/\* [0-9]+ \*/');
+next if (/^\s+$/);
+if ($counter % $step == 0) {
+if ($counter != 0) {
+push @fixed, "\n";
+}
+push @fixed, "$counter_prefix/* $counter */\n";
+}
+if (!($_ =~ '/\*' && !($_ =~ '\*/'))) {
+# count two-line comments as one line
+$counter++;
+}
+}
+if (/$start_regex/) {
+$game_on = 1;
+} elsif ($game_on && $_ =~ /$end_regex/) {
+if (($counter -1) % $step == 0) {
+pop @fixed; # /* $counter */
+if ($counter != 1) {
+pop @fixed; # \n
+}
+}
+if ($trailing_newline) {
+push @fixed, "\n";
+}
+$game_on = 0;
+}
+push @fixed, $_;
+}
+
+if ($check) {
+my