On Wed, Jun 24, 2015 at 06:17:35PM +0200, Johannes Holmberg wrote:
----- On 24 Jun, 2015, at 09:14, Martin Kletzander [email protected] wrote:
Why don't you just set XMLFILE="$TMPFILE" ?  That would get rid of lot
of the code changes below and would be more readable.

Other than that it looks good.

Good question, with a somewhat long answer. I initially did exactly what you suggest, and yes, it 
makes for simpler code. The only problem with it is that when xmllint finds a problem with the xml, 
it will output a string on the format "file name:line number: error description". If the 
file was provided on stdin, the file name will be something like "/tmp/virt-xml.asdf" 
which will not make much sense to the user.
So instead I tried to be consistent: if virt-xml-validate received a filename, 
xmllint will receive a filename. If virt-xml-validate received data on stdin, 
xmllint will receive data on stdin.
This way, when the xml is provided on stdin and xmllint finds a problem, the file name in 
the error message will just show up as "-", which is probably more in line with 
what the user was expecting.

Anyway, it's not something I feel super-strongly about, so if you prefer to 
have it changed, just let me know and I'll have an updated patch for you.


I'm on the edge here.  You've got perfectly good point here.  If there
was at least a way of simplifying each condition somehow.  Each such
approach pollutes the code with 'eval' or new function that makes it
even more unreadable :(
Anyway, I'm going into too much unnecessary details.  Thanks to your
explanation I'm OK with your approach, I would suggest just one teeny
tiny change, and that's using 'xmllint - < $file' instead of
'cat $file | xmllint -'.

If that's ok with you, just Cc me on the v2 and I'll have a look at
it.

Thanks,
Martin

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to