[libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

--- Hello, This is an updated version of a patch I submitted on 2015-06-10. I got some feedback on it but then moved on to a different project and forgot about it. Anyway, I've updated the patch according to the feedback so if you are still interested, here it is! :) /Johannes tools/virt-xml-validate.in | 44 ++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 64aeaaaa33..2d2afb74ec 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -16,6 +16,16 @@ set -e +TMPFILE= + +cleanup() { + if [ $TMPFILE ]; then + rm -f $TMPFILE + fi +} + +trap cleanup EXIT + case $1 in -h | --h | --he | --hel | --help) cat <<EOF @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@ EOF exit ;; --) shift ;; - -*) + -?*) echo "$0: unrecognized option '$1'" >&2 exit 1 ;; esac @@ -42,18 +52,27 @@ esac XMLFILE="$1" TYPE="$2" -if [ -z "$XMLFILE" ]; then - echo "syntax: $0 XMLFILE [TYPE]" >&2 - exit 1 -fi +if [ "$XMLFILE" = "-" ]; then + TMPFILE=`mktemp --tmpdir virt-xml.XXXX` + cat > $TMPFILE +else + if [ -z "$XMLFILE" ]; then + echo "syntax: $0 XMLFILE [TYPE]" >&2 + exit 1 + fi -if [ ! -f "$XMLFILE" ]; then - echo "$0: document $XMLFILE does not exist" >&2 - exit 2 + if [ ! -f "$XMLFILE" ]; then + echo "$0: document $XMLFILE does not exist" >&2 + exit 2 + fi fi if [ -z "$TYPE" ]; then - ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + if [ $TMPFILE ]; then + ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + else + ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + fi case "$ROOT" in *domainsnapshot*) # Must come first, since *domain* is a substring TYPE="domainsnapshot" @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then exit 4 fi -xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" - +if [ $TMPFILE ]; then + xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE" +else + xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" +fi exit -- 2.17.1

On Mon, May 20, 2019 at 11:57:13AM +0000, Johannes Holmberg wrote:
---
Hello,
This is an updated version of a patch I submitted on 2015-06-10. I got some feedback on it but then moved on to a different project and forgot about it. Anyway, I've updated the patch according to the feedback so if you are still interested, here it is! :)
Hi, thanks for the patch. Wouldn't it be easier and cleaner if we just did: if [ "$XMLFILE" = "-" ]; then XMLFILE=/dev/stdin fi ??

On Mon, 2019-05-20 at 14:18 +0200, Martin Kletzander wrote:
On Mon, May 20, 2019 at 11:57:13AM +0000, Johannes Holmberg wrote:
---
Hello,
This is an updated version of a patch I submitted on 2015-06-10. I got some feedback on it but then moved on to a different project and forgot about it. Anyway, I've updated the patch according to the feedback so if you are still interested, here it is! :)
Hi, thanks for the patch. Wouldn't it be easier and cleaner if we just did:
if [ "$XMLFILE" = "-" ]; then XMLFILE=/dev/stdin fi
??
I tried it but the problem is that if the SCHEMA-NAME argument is not supplied, the validation needs two passes, one to read the root tag, and another one to do the actual xml validation. If I use /dev/stdin the second pass will (sometimes) fail because the input is already exhausted. It works if the input is redirected from a file: $ ./virt-xml-validate - < asdf.xml /dev/stdin validates But if I pipe it from another program it fails: $ cat asdf.xml | ./virt-xml-validate - /dev/stdin:1: parser error : Document is empty /Johannes

Hello, Some nits: Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 64aeaaaa33..2d2afb74ec 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -16,6 +16,16 @@
set -e
+TMPFILE= + +cleanup() { + if [ $TMPFILE ]; then
Missing Quoting. Better also give "-n"
+ rm -f $TMPFILE
Quoting
+ fi +} + +trap cleanup EXIT + case $1 in
Not your fault, but also missing quoting.
-h | --h | --he | --hel | --help) cat <<EOF @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@ EOF exit ;; --) shift ;; - -*) + -?*) echo "$0: unrecognized option '$1'" >&2 exit 1 ;; esac @@ -42,18 +52,27 @@ esac XMLFILE="$1" TYPE="$2"
-if [ -z "$XMLFILE" ]; then - echo "syntax: $0 XMLFILE [TYPE]" >&2 - exit 1 -fi +if [ "$XMLFILE" = "-" ]; then + TMPFILE=`mktemp --tmpdir virt-xml.XXXX`> + cat > $TMPFILE
Quoting
+else + if [ -z "$XMLFILE" ]; then + echo "syntax: $0 XMLFILE [TYPE]" >&2 + exit 1 + fi
-if [ ! -f "$XMLFILE" ]; then - echo "$0: document $XMLFILE does not exist" >&2 - exit 2 + if [ ! -f "$XMLFILE" ]; then + echo "$0: document $XMLFILE does not exist" >&2 + exit 2 + fi fi
if [ -z "$TYPE" ]; then - ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + if [ $TMPFILE ]; then + ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + else + ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + fi case "$ROOT" in *domainsnapshot*) # Must come first, since *domain* is a substring TYPE="domainsnapshot" @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then exit 4 fi
-xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" - +if [ $TMPFILE ]; then
Quoting "-n"
+ xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE" +else + xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" +fi exit
Philipp

On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote:
Hello,
Some nits:
Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml- validate.in index 64aeaaaa33..2d2afb74ec 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -16,6 +16,16 @@
set -e
+TMPFILE= + +cleanup() { + if [ $TMPFILE ]; then
Missing Quoting. Better also give "-n"
+ rm -f $TMPFILE
Quoting
+ fi +} + +trap cleanup EXIT + case $1 in
Not your fault, but also missing quoting.
-h | --h | --he | --hel | --help) cat <<EOF @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@ EOF exit ;; --) shift ;; - -*) + -?*) echo "$0: unrecognized option '$1'" >&2 exit 1 ;; esac @@ -42,18 +52,27 @@ esac XMLFILE="$1" TYPE="$2"
-if [ -z "$XMLFILE" ]; then - echo "syntax: $0 XMLFILE [TYPE]" >&2 - exit 1 -fi +if [ "$XMLFILE" = "-" ]; then + TMPFILE=`mktemp --tmpdir virt-xml.XXXX`> + cat > $TMPFILE
Quoting
+else + if [ -z "$XMLFILE" ]; then + echo "syntax: $0 XMLFILE [TYPE]" >&2 + exit 1 + fi
-if [ ! -f "$XMLFILE" ]; then - echo "$0: document $XMLFILE does not exist" >&2 - exit 2 + if [ ! -f "$XMLFILE" ]; then + echo "$0: document $XMLFILE does not exist" >&2 + exit 2 + fi fi
if [ -z "$TYPE" ]; then - ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + if [ $TMPFILE ]; then + ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + else + ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + fi case "$ROOT" in *domainsnapshot*) # Must come first, since *domain* is a substring TYPE="domainsnapshot" @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then exit 4 fi
-xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" - +if [ $TMPFILE ]; then
Quoting "-n"
+ xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE" +else + xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" +fi exit
Good point(s). I can only imagine I skipped the quoting because TMPFILE is known to contain a safe string, but that is a poor excuse really. I'll post a v2 shortly. /Johannes

Hello, Am 21.05.19 um 09:35 schrieb Johannes Holmberg:
On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote:
Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in ... +if [ "$XMLFILE" = "-" ]; then + TMPFILE=`mktemp --tmpdir virt-xml.XXXX` ... Quoting
Good point(s). I can only imagine I skipped the quoting because TMPFILE is known to contain a safe string,
No, it's not guaranteed, as it honors TMPDIR:
$ LANG=C TMPDIR='/tmp/un safe' mktemp --tmpdir virt-xml.XXXX mktemp: failed to create file via template '/tmp/un safe/virt-xml.XXXX': ...
Philipp
participants (3)
-
Johannes Holmberg
-
Martin Kletzander
-
Philipp Hahn