6:57 a.m.
---
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
7:18 a.m.
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
??
8:02 a.m.
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
12:36 a.m.
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
2:35 a.m.
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
2:41 a.m.
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
2013
days inactive
2014
days old
5 comments
3 participants
participants (3)
-
Johannes Holmberg -
Martin Kletzander -
Philipp Hahn