Re: [libvirt] [PATCH v2 3/5] conf: Make virNetworkIPDefParseXML a little bit saner
On Wed, Dec 14, 2016 at 10:46:20 -0500, Laine Stump wrote:
On 12/13/2016 08:52 AM, Jiri Denemark wrote:
> Iterating over all child nodes when we only support one instance of each
> child is pretty weired. And it would even cause memory leaks if more
> than one <tftp> element was specified.
ACK, but could you also look for dhcp[2]/tftp[2] and log an error if found?
I know there are *lots* of places we ignore extra elements in the XML, but
in this case there would be a silent behavior change if someone had
(erroneous) multiple tftp or dhcp elements - previously we would have
honored the final occurence of each element, but now we honor the first. So
even though it's their own fault, it would be nice to
Well, if a user provided such a wrong XML, libvirt didn't do what the
user asked for anyway. I don't think implementing a different way of not
doing what they asked for is something we should worry about :-)
That said, adding the check there is trivial, but I'm not sure it's a
good idea since we do not check for extra elements anywhere else.
(BTW, I've always disliked that some of our XML parsing code
iterates
through the raw nodes like this used to, and some uses XPath to get specific
nodes. It would be so much easier for newcomers to understand if we picked
on method and used it consistently...)
Yes, I think we'll eventually rewrite all parsing to use XPath, but it
will take some time.
Jirka