[libvirt] [PATCH 0/2] conf: Avoid checking root element name in virDomainDefParseNode

The only caller for which this check makes sense is virDomainDefParse. Thus the check should be moved there. Jiri Denemark (2): conf: Add cleanup label to virDomainDefParse conf: Avoid checking root element name in virDomainDefParseNode src/conf/domain_conf.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) -- 2.23.0

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f49c8253f..17ddebb575 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21517,16 +21517,18 @@ virDomainDefParse(const char *xmlStr, void *parseOpaque, unsigned int flags) { - xmlDocPtr xml; + xmlDocPtr xml = NULL; virDomainDefPtr def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(filename, xmlStr, _("(domain_definition)")))) { - def = virDomainDefParseNode(xml, xmlDocGetRootElement(xml), caps, - xmlopt, parseOpaque, flags); - xmlFreeDoc(xml); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)")))) + goto cleanup; + def = virDomainDefParseNode(xml, xmlDocGetRootElement(xml), caps, + xmlopt, parseOpaque, flags); + + cleanup: + xmlFreeDoc(xml); xmlKeepBlanksDefault(keepBlanksDefault); return def; } -- 2.23.0

The only caller for which this check makes sense is virDomainDefParse. Thus the check should be moved there. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 17ddebb575..b3b8c543d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21520,12 +21520,21 @@ virDomainDefParse(const char *xmlStr, xmlDocPtr xml = NULL; virDomainDefPtr def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); + xmlNodePtr root; if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)")))) goto cleanup; - def = virDomainDefParseNode(xml, xmlDocGetRootElement(xml), caps, - xmlopt, parseOpaque, flags); + root = xmlDocGetRootElement(xml); + if (!virXMLNodeNameEqual(root, "domain")) { + virReportError(VIR_ERR_XML_ERROR, + _("unexpected root element <%s>, " + "expecting <domain>"), + root->name); + goto cleanup; + } + + def = virDomainDefParseNode(xml, root, caps, xmlopt, parseOpaque, flags); cleanup: xmlFreeDoc(xml); @@ -21566,14 +21575,6 @@ virDomainDefParseNode(xmlDocPtr xml, virDomainDefPtr def = NULL; virDomainDefPtr ret = NULL; - if (!virXMLNodeNameEqual(root, "domain")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <domain>"), - root->name); - goto cleanup; - } - ctxt = xmlXPathNewContext(xml); if (ctxt == NULL) { virReportOOMError(); -- 2.23.0

On 9/10/19 10:45 AM, Jiri Denemark wrote:
The only caller for which this check makes sense is virDomainDefParse. Thus the check should be moved there.
Jiri Denemark (2): conf: Add cleanup label to virDomainDefParse conf: Avoid checking root element name in virDomainDefParseNode
src/conf/domain_conf.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jiri Denemark
-
Michal Privoznik