11:39 a.m.
The --in-place parameter makes the invocation in testutils slightly nicer
and --check makes syntax-check faster.
Ján Tomko (8):
test-wrap-argv: split out rewrap_line
test-wrap-argv: split out rewrap_arg
test-wrap-argv: return a string in rewrap_arg
test-wrap-argv: use map and join instead of a for cycle
test-wrap-argv: return a string in rewrap_line
test-wrap-argv: hold a copy of the original file in an array
test-wrap-argv: add --in-place parameter
test-wrap-argv: add --check parameter
cfg.mk | 12 +---
tests/test-wrap-argv.pl | 170 ++++++++++++++++++++++++++++++------------------
tests/testutils.c | 8 +--
3 files changed, 108 insertions(+), 82 deletions(-)
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 1/8] test-wrap-argv: split out rewrap_line
Shorten the rewrap subroutine by splitting out the code
dealing with a single line.
Also remove $file from the warning.
---
tests/test-wrap-argv.pl | 116 +++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 55 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index a81cf43..3847d7b 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -59,70 +59,76 @@ sub rewrap {
# Now each @lines represents a single command, we
# can process them
foreach my $line (@lines) {
- my @bits = split / /, join('', $line);
+ &rewrap_line ($line);
+ }
+
+}
- # @bits contains env vars, then the command line
- # and then the arguments
- my @env;
- my $cmd;
- my @args;
+sub rewrap_line {
+ my $line = shift;
+ my @bits = split / /, join('', $line);
- if ($bits[0] !~ /=/) {
- $cmd = shift @bits;
- }
+ # @bits contains env vars, then the command line
+ # and then the arguments
+ my @env;
+ my $cmd;
+ my @args;
- foreach my $bit (@bits) {
- # If no command is defined yet, we must still
- # have env vars
- if (!defined $cmd) {
- # Look for leading / to indicate command name
- if ($bit =~ m,^/,) {
- $cmd = $bit;
- } else {
- push @env, $bit;
- }
+ if ($bits[0] !~ /=/) {
+ $cmd = shift @bits;
+ }
+
+ foreach my $bit (@bits) {
+ # If no command is defined yet, we must still
+ # have env vars
+ if (!defined $cmd) {
+ # Look for leading / to indicate command name
+ if ($bit =~ m,^/,) {
+ $cmd = $bit;
+ } else {
+ push @env, $bit;
+ }
+ } else {
+ # If there's a leading '-' then this is a new
+ # parameter, otherwise its a value for the prev
+ # parameter.
+ if ($bit =~ m,^-,) {
+ push @args, $bit;
} else {
- # If there's a leading '-' then this is a new
- # parameter, otherwise its a value for the prev
- # parameter.
- if ($bit =~ m,^-,) {
- push @args, $bit;
- } else {
- $args[$#args] .= " " . $bit;
- }
+ $args[$#args] .= " " . $bit;
}
}
+ }
- # Print env + command first
- print join(" \\\n", @env, $cmd), " \\\n";
- # We might have to split line argument values...
- for (my $i = 0; $i <= $#args; $i++) {
- my $arg = $args[$i];
- while (length($arg) > 80) {
- my $split = rindex $arg, ",", 80;
- if ($split == -1) {
- $split = rindex $arg, ":", 80;
- }
- if ($split == -1) {
- $split = rindex $arg, " ", 80;
- }
- if ($split == -1) {
- warn "$file: cannot find nice place to split '$arg'
below 80 chars\n";
- $split = 79;
- }
- $split++;
-
- my $head = substr $arg, 0, $split;
- $arg = substr $arg, $split;
-
- print $head, "\\\n";
+ # Print env + command first
+ print join(" \\\n", @env, $cmd), " \\\n";
+ # We might have to split line argument values...
+ for (my $i = 0; $i <= $#args; $i++) {
+ my $arg = $args[$i];
+ while (length($arg) > 80) {
+ my $split = rindex $arg, ",", 80;
+ if ($split == -1) {
+ $split = rindex $arg, ":", 80;
}
- print $arg;
- if ($i != $#args) {
- print " \\\n";
- } else {
- print "\n";
+ if ($split == -1) {
+ $split = rindex $arg, " ", 80;
+ }
+ if ($split == -1) {
+ warn "cannot find nice place to split '$arg' below 80
chars\n";
+ $split = 79;
}
+ $split++;
+
+ my $head = substr $arg, 0, $split;
+ $arg = substr $arg, $split;
+
+ print $head, "\\\n";
+ }
+ print $arg;
+ if ($i != $#args) {
+ print " \\\n";
+ } else {
+ print "\n";
}
}
}
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 2/8] test-wrap-argv: split out rewrap_arg
Split out the code wrapping the single argument.
---
tests/test-wrap-argv.pl | 46 ++++++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 3847d7b..72a3e32 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -104,27 +104,8 @@ sub rewrap_line {
print join(" \\\n", @env, $cmd), " \\\n";
# We might have to split line argument values...
for (my $i = 0; $i <= $#args; $i++) {
- my $arg = $args[$i];
- while (length($arg) > 80) {
- my $split = rindex $arg, ",", 80;
- if ($split == -1) {
- $split = rindex $arg, ":", 80;
- }
- if ($split == -1) {
- $split = rindex $arg, " ", 80;
- }
- if ($split == -1) {
- warn "cannot find nice place to split '$arg' below 80
chars\n";
- $split = 79;
- }
- $split++;
-
- my $head = substr $arg, 0, $split;
- $arg = substr $arg, $split;
+ &rewrap_arg($args[$i]);
- print $head, "\\\n";
- }
- print $arg;
if ($i != $#args) {
print " \\\n";
} else {
@@ -132,3 +113,28 @@ sub rewrap_line {
}
}
}
+
+sub rewrap_arg {
+ my $arg = shift;
+
+ while (length($arg) > 80) {
+ my $split = rindex $arg, ",", 80;
+ if ($split == -1) {
+ $split = rindex $arg, ":", 80;
+ }
+ if ($split == -1) {
+ $split = rindex $arg, " ", 80;
+ }
+ if ($split == -1) {
+ warn "cannot find nice place to split '$arg' below 80
chars\n";
+ $split = 79;
+ }
+ $split++;
+
+ my $head = substr $arg, 0, $split;
+ $arg = substr $arg, $split;
+
+ print $head, "\\\n";
+ }
+ print $arg;
+}
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 3/8] test-wrap-argv: return a string in rewrap_arg
Do not print anything, let the caller take care of it.
---
tests/test-wrap-argv.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 72a3e32..693bed5 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -104,7 +104,7 @@ sub rewrap_line {
print join(" \\\n", @env, $cmd), " \\\n";
# We might have to split line argument values...
for (my $i = 0; $i <= $#args; $i++) {
- &rewrap_arg($args[$i]);
+ print &rewrap_arg($args[$i]);
if ($i != $#args) {
print " \\\n";
@@ -116,6 +116,7 @@ sub rewrap_line {
sub rewrap_arg {
my $arg = shift;
+ my @ret;
while (length($arg) > 80) {
my $split = rindex $arg, ",", 80;
@@ -131,10 +132,9 @@ sub rewrap_arg {
}
$split++;
- my $head = substr $arg, 0, $split;
+ push @ret, substr $arg, 0, $split;
$arg = substr $arg, $split;
-
- print $head, "\\\n";
}
- print $arg;
+ push @ret, $arg;
+ return join("\\\n", @ret);
}
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 4/8] test-wrap-argv: use map and join instead of a for cycle
We have a list of parameters in @args, that need to be rewrapped
and separated by a space and escaped newline: " \\\n", with the
exception of the last one, which only needs a newline.
Instead of a for cycle, rewrap the individual arguments using map,
and interleave them with escaped newlines by using join.
---
tests/test-wrap-argv.pl | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 693bed5..4e942cd 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -100,18 +100,10 @@ sub rewrap_line {
}
}
- # Print env + command first
- print join(" \\\n", @env, $cmd), " \\\n";
# We might have to split line argument values...
- for (my $i = 0; $i <= $#args; $i++) {
- print &rewrap_arg($args[$i]);
-
- if ($i != $#args) {
- print " \\\n";
- } else {
- print "\n";
- }
- }
+ @args = map { &rewrap_arg($_) } @args;
+ # Print env + command first
+ print join(" \\\n", @env, $cmd, @args), "\n";
}
sub rewrap_arg {
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 5/8] test-wrap-argv: return a string in rewrap_line
Leave the printing up to &rewrap.
---
tests/test-wrap-argv.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 4e942cd..1f619cc 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -59,7 +59,7 @@ sub rewrap {
# Now each @lines represents a single command, we
# can process them
foreach my $line (@lines) {
- &rewrap_line ($line);
+ print &rewrap_line ($line);
}
}
@@ -103,7 +103,7 @@ sub rewrap_line {
# We might have to split line argument values...
@args = map { &rewrap_arg($_) } @args;
# Print env + command first
- print join(" \\\n", @env, $cmd, @args), "\n";
+ return join(" \\\n", @env, $cmd, @args), "\n";
}
sub rewrap_arg {
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 6/8] test-wrap-argv: hold a copy of the original file in an array
This will be useful to check if the file is wrapped already.
---
tests/test-wrap-argv.pl | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 1f619cc..96f998a 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -33,15 +33,15 @@ sub rewrap {
# Read the original file
open FILE, "<", $file or die "cannot read $file: $!";
- my @lines;
- while (<FILE>) {
+ my @orig_lines = <FILE>;
+ close FILE;
+ my @lines = @orig_lines;
+ foreach (@lines) {
# If there is a trailing '\' then kill the new line
if (/\\$/) {
chomp;
$_ =~ s/\\$//;
}
-
- push @lines, $_;
}
# Skip empty files
@@ -49,7 +49,6 @@ sub rewrap {
# Kill the last new line in the file
chomp @lines[$#lines];
- close FILE;
# Reconstruct the master data by joining all lines
# and then split again based on the real desired
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 7/8] test-wrap-argv: add --in-place parameter
If --in-place is supplied as the first argument to the script,
replace the file in-place instead of printing to stdout.
---
tests/test-wrap-argv.pl | 24 +++++++++++++++++++++---
tests/testutils.c | 8 +-------
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 96f998a..97f6903 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -22,7 +22,16 @@
# of the file. Parameter values that are longer than 80 chars will
# also be split.
#
+# If --in-place is supplied as the first parameter of this script,
+# the files will be changed in place.
+# Otherwise the rewrapped files are printed to the standard output.
+$in_place = 0;
+
+if (@ARGV[0] eq "--in-place") {
+ $in_place = 1;
+ shift @ARGV;
+}
foreach my $file (@ARGV) {
&rewrap($file);
@@ -57,10 +66,19 @@ sub rewrap {
# Now each @lines represents a single command, we
# can process them
- foreach my $line (@lines) {
- print &rewrap_line ($line);
- }
+ @lines = map { &rewrap_line($_) } @lines;
+ if ($in_place) {
+ open FILE, ">", $file or die "cannot write $file: $!";
+ foreach my $line (@lines) {
+ print FILE $line;
+ }
+ close FILE;
+ } else {
+ foreach my $line (@lines) {
+ print $line;
+ }
+ }
}
sub rewrap_line {
diff --git a/tests/testutils.c b/tests/testutils.c
index 54adab2..1d503c1 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -440,26 +440,20 @@ static int
virTestRewrapFile(const char *filename)
{
int ret = -1;
- char *outbuf = NULL;
char *script = NULL;
virCommandPtr cmd = NULL;
if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
goto cleanup;
- cmd = virCommandNewArgList(script, filename, NULL);
- virCommandSetOutputBuffer(cmd, &outbuf);
+ cmd = virCommandNewArgList(script, "--in-place", filename, NULL);
if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
- if (virFileWriteStr(filename, outbuf, 0666) < 0)
- goto cleanup;
-
ret = 0;
cleanup:
VIR_FREE(script);
virCommandFree(cmd);
- VIR_FREE(outbuf);
return ret;
}
--
2.7.3
11:39 a.m.
New subject: [libvirt] [PATCH 8/8] test-wrap-argv: add --check parameter
This script can already operate on a list of files.
Add a --check parameter to check if multiple files are wrapped
correctly with a single invocation of the script.
---
cfg.mk | 12 +-----------
tests/test-wrap-argv.pl | 23 ++++++++++++++++++++++-
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 297ca3a..f48c035 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1108,17 +1108,7 @@ spacing-check:
test-wrap-argv:
$(AM_V_GEN)files=`$(VC_LIST) | grep -E '\.(ldargs|args)'`; \
- for file in $$files ; \
- do \
- $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl $$file > $${file}-t ; \
- diff $$file $${file}-t; \
- res=$$? ; \
- rm $${file}-t ; \
- test $$res == 0 || { \
- echo "$(ME): Incorrect line wrapping in $$file" 1>&2; \
- echo "$(ME): Use test-wrap-argv.pl to wrap test data files"
1>&2; \
- exit 1; } \
- done
+ $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check $$files
# sc_po_check can fail if generated files are not built first
sc_po_check: \
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
index 97f6903..d66f5b4 100755
--- a/tests/test-wrap-argv.pl
+++ b/tests/test-wrap-argv.pl
@@ -24,19 +24,30 @@
#
# If --in-place is supplied as the first parameter of this script,
# the files will be changed in place.
+# If --check is the first parameter, the script will return
+# a non-zero value if a file is not wrapped correctly.
# Otherwise the rewrapped files are printed to the standard output.
$in_place = 0;
+$check = 0;
if (@ARGV[0] eq "--in-place") {
$in_place = 1;
shift @ARGV;
+} elsif (@ARGV[0] eq "--check") {
+ $check = 1;
+ shift @ARGV;
}
foreach my $file (@ARGV) {
- &rewrap($file);
+ $ret = 0;
+ if (&rewrap($file) < 0) {
+ $ret = 1;
+ }
}
+exit $ret;
+
sub rewrap {
my $file = shift;
@@ -74,11 +85,21 @@ sub rewrap {
print FILE $line;
}
close FILE;
+ } elsif ($check) {
+ my $nl = join('', @lines);
+ my $ol = join('', @orig_lines);
+ unless ($nl eq $ol) {
+ print STDERR $ol;
+ print STDERR "Incorrect line wrapping in $file\n";
+ print STDERR "Use test-wrap-argv.pl to wrap test data files\n";
+ return -1;
+ }
} else {
foreach my $line (@lines) {
print $line;
}
}
+ return 0;
}
sub rewrap_line {
--
2.7.3
5:41 a.m.
On 06/15/2016 12:39 PM, Ján Tomko wrote:
The --in-place parameter makes the invocation in testutils slightly
nicer
and --check makes syntax-check faster.
Ján Tomko (8):
test-wrap-argv: split out rewrap_line
test-wrap-argv: split out rewrap_arg
test-wrap-argv: return a string in rewrap_arg
test-wrap-argv: use map and join instead of a for cycle
test-wrap-argv: return a string in rewrap_line
test-wrap-argv: hold a copy of the original file in an array
test-wrap-argv: add --in-place parameter
test-wrap-argv: add --check parameter
cfg.mk | 12 +---
tests/test-wrap-argv.pl | 170 ++++++++++++++++++++++++++++++------------------
tests/testutils.c | 8 +--
3 files changed, 108 insertions(+), 82 deletions(-)
I believe one other difference is that "all" the differences or issues
are generated and displayed not just one at a time (which I actually
prefer).
Changes look OK to me; however, I think an "existing" issue is that the
splitting is off by 1 or 2 characters. If a ",", ":", or " "
is the 80th
character and then a " \" is added, that addition is on the next line if
you have a an 80 column display (likewise a "\" when there are more args
to be printed).
Check out the following files and you'll note by visual inspection there
are lines that are longer than 80:
tests/qemuargv2xmldata/qemuargv2xml-disk-drive-cache-unsafe.args
tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-addr.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args
All I did was change 79 to 78 and 80 to 79 (it gets worse with 78/77)
ACK to what's done though...
John
11:15 a.m.
On Tue, Jun 21, 2016 at 06:41:35AM -0400, John Ferlan wrote:
On 06/15/2016 12:39 PM, Ján Tomko wrote:
> The --in-place parameter makes the invocation in testutils slightly nicer
> and --check makes syntax-check faster.
>
> Ján Tomko (8):
> test-wrap-argv: split out rewrap_line
> test-wrap-argv: split out rewrap_arg
> test-wrap-argv: return a string in rewrap_arg
> test-wrap-argv: use map and join instead of a for cycle
> test-wrap-argv: return a string in rewrap_line
> test-wrap-argv: hold a copy of the original file in an array
> test-wrap-argv: add --in-place parameter
> test-wrap-argv: add --check parameter
>
> cfg.mk | 12 +---
> tests/test-wrap-argv.pl | 170 ++++++++++++++++++++++++++++++------------------
> tests/testutils.c | 8 +--
> 3 files changed, 108 insertions(+), 82 deletions(-)
>
I believe one other difference is that "all" the differences or issues
are generated and displayed not just one at a time (which I actually
prefer).
Changes look OK to me; however, I think an "existing" issue is that the
splitting is off by 1 or 2 characters. If a ",", ":", or " "
is the 80th
character and then a " \" is added, that addition is on the next line if
you have a an 80 column display (likewise a "\" when there are more args
to be printed).
Check out the following files and you'll note by visual inspection there
are lines that are longer than 80:
tests/qemuargv2xmldata/qemuargv2xml-disk-drive-cache-unsafe.args
tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-addr.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args
All I did was change 79 to 78 and 80 to 79 (it gets worse with 78/77)
ACK to what's done though...
Thanks, I've pushed the series and marked the issues on my TODO list.
Jan
3076
days inactive
3082
days old
10 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko