[libvirt] [PATCH 0/3] xml simplifications
I noticed this while working on my snapshot stuff. There was lots of copy and paste going on in virsh.c, and it felt awful to not be able to represent the same idea in fewer lines, so I fixed it. This will probably mean I have to rebase the entire snapshot series, since it will introduce several conflicts in at least virsh.c; oh well. Eric Blake (3): maint: treat more libxml2 functions as free-like xml: add another convenience function maint: simplify lots of libxml2 clients cfg.mk | 2 + src/conf/domain_conf.c | 20 +------ src/conf/storage_conf.c | 8 +--- src/cpu/cpu.c | 18 +----- src/esx/esx_vi.c | 17 ++----- src/libvirt_private.syms | 2 - src/qemu/qemu_migration.c | 9 +--- src/security/virt-aa-helper.c | 12 +---- src/test/test_driver.c | 13 +---- src/util/xml.c | 53 +++++------------- src/util/xml.h | 84 +++++++++++++++++++++++++--- tests/cputest.c | 9 +--- tools/virsh.c | 120 +++++++++-------------------------------- 13 files changed, 135 insertions(+), 232 deletions(-) -- 1.7.4.4
* cfg.mk (useless_free_options): Add xmlFreeDoc, xmlBufferFree. * src/esx/esx_vi.c (ESX_VI__TEMPLATE__FREE): Fix offenders. * tools/virsh.c (cmdFreecell, cmdVNCDisplay, cmdTTYConsole) (cmdDetachInterface, cmdDetachDisk, cmdSnapshotCreate) (cmdSnapshotCreateAs, cmdSnapshotList, cmdSnapshotParent): Likewise. --- cfg.mk | 2 ++ src/esx/esx_vi.c | 4 +--- tools/virsh.c | 36 ++++++++++++------------------------ 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/cfg.mk b/cfg.mk index 2eb73e6..9f1f6c5 100644 --- a/cfg.mk +++ b/cfg.mk @@ -165,7 +165,9 @@ useless_free_options = \ --name=virStoragePoolSourceFree \ --name=virStorageVolDefFree \ --name=virThreadPoolFree \ + --name=xmlBufferFree \ --name=xmlFree \ + --name=xmlFreeDoc \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 64e5b73..c82094b 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -1074,9 +1074,7 @@ ESX_VI__TEMPLATE__FREE(Response, { VIR_FREE(item->content); - if (item->document != NULL) { - xmlFreeDoc(item->document); - } + xmlFreeDoc(item->document); }) diff --git a/tools/virsh.c b/tools/virsh.c index f1eb4ca..c43de4c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2971,8 +2971,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(nodes); VIR_FREE(nodes_free); VIR_FREE(nodes_id); @@ -10234,8 +10233,7 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); virDomainFree(dom); return ret; } @@ -10295,8 +10293,7 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); virDomainFree(dom); return ret; } @@ -10752,10 +10749,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) virDomainFree(dom); xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); - if (xml_buf) - xmlBufferFree(xml_buf); + xmlFreeDoc(xml); + xmlBufferFree(xml_buf); return functionReturn; } @@ -11216,10 +11211,8 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); - if (xml_buf) - xmlBufferFree(xml_buf); + xmlFreeDoc(xml); + xmlBufferFree(xml_buf); if (dom) virDomainFree(dom); return functionReturn; @@ -11898,8 +11891,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(name); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); if (snapshot) virDomainSnapshotFree(snapshot); VIR_FREE(doc); @@ -12005,8 +11997,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(parsed_name); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); if (snapshot) virDomainSnapshotFree(snapshot); VIR_FREE(doc); @@ -12163,8 +12154,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (snapshot) virDomainSnapshotFree(snapshot); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(doc); snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); @@ -12210,8 +12200,7 @@ cleanup: if (snapshot) virDomainSnapshotFree(snapshot); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(doc); for (i = 0; i < actual; i++) VIR_FREE(names[i]); @@ -12343,8 +12332,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(parent); xmlXPathFreeContext(ctxt); - if (xmldoc) - xmlFreeDoc(xmldoc); + xmlFreeDoc(xmldoc); VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); -- 1.7.4.4
On Thu, Aug 18, 2011 at 03:40:47PM -0600, Eric Blake wrote:
* cfg.mk (useless_free_options): Add xmlFreeDoc, xmlBufferFree. * src/esx/esx_vi.c (ESX_VI__TEMPLATE__FREE): Fix offenders. * tools/virsh.c (cmdFreecell, cmdVNCDisplay, cmdTTYConsole) (cmdDetachInterface, cmdDetachDisk, cmdSnapshotCreate) (cmdSnapshotCreateAs, cmdSnapshotList, cmdSnapshotParent): Likewise. --- cfg.mk | 2 ++ src/esx/esx_vi.c | 4 +--- tools/virsh.c | 36 ++++++++++++------------------------ 3 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/cfg.mk b/cfg.mk index 2eb73e6..9f1f6c5 100644 --- a/cfg.mk +++ b/cfg.mk @@ -165,7 +165,9 @@ useless_free_options = \ --name=virStoragePoolSourceFree \ --name=virStorageVolDefFree \ --name=virThreadPoolFree \ + --name=xmlBufferFree \ --name=xmlFree \ + --name=xmlFreeDoc \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 64e5b73..c82094b 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -1074,9 +1074,7 @@ ESX_VI__TEMPLATE__FREE(Response, { VIR_FREE(item->content);
- if (item->document != NULL) { - xmlFreeDoc(item->document); - } + xmlFreeDoc(item->document); })
diff --git a/tools/virsh.c b/tools/virsh.c index f1eb4ca..c43de4c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2971,8 +2971,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
cleanup: xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(nodes); VIR_FREE(nodes_free); VIR_FREE(nodes_id); @@ -10234,8 +10233,7 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); virDomainFree(dom); return ret; } @@ -10295,8 +10293,7 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); virDomainFree(dom); return ret; } @@ -10752,10 +10749,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) virDomainFree(dom); xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); - if (xml_buf) - xmlBufferFree(xml_buf); + xmlFreeDoc(xml); + xmlBufferFree(xml_buf); return functionReturn; }
@@ -11216,10 +11211,8 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); - if (xml_buf) - xmlBufferFree(xml_buf); + xmlFreeDoc(xml); + xmlBufferFree(xml_buf); if (dom) virDomainFree(dom); return functionReturn; @@ -11898,8 +11891,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(name); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); if (snapshot) virDomainSnapshotFree(snapshot); VIR_FREE(doc); @@ -12005,8 +11997,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(parsed_name); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); if (snapshot) virDomainSnapshotFree(snapshot); VIR_FREE(doc); @@ -12163,8 +12154,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (snapshot) virDomainSnapshotFree(snapshot); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(doc);
snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); @@ -12210,8 +12200,7 @@ cleanup: if (snapshot) virDomainSnapshotFree(snapshot); xmlXPathFreeContext(ctxt); - if (xml) - xmlFreeDoc(xml); + xmlFreeDoc(xml); VIR_FREE(doc); for (i = 0; i < actual; i++) VIR_FREE(names[i]); @@ -12343,8 +12332,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(parent); xmlXPathFreeContext(ctxt); - if (xmldoc) - xmlFreeDoc(xmldoc); + xmlFreeDoc(xmldoc); VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot);
ACK, the libxml2 entry points check for NULL, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Often, we want to use XPath functions on the just-parsed document; fold this into the parser function for convenience. * src/util/xml.h (virXMLParseHelper): Add argument. (virXMLParseStrHelper, virXMLParseFileHelper): Delete. (virXMLParseCtxt, virXMLParseStringCtxt, virXMLParseFileCtxt): New macros. * src/libvirt_private.syms (xml.h): Remove deleted functions. * src/util/xml.c (virXMLParseHelper): Add argument. (virXMLParseStrHelper, virXMLParseFileHelper): Delete. --- src/libvirt_private.syms | 2 - src/util/xml.c | 53 ++++++++--------------------- src/util/xml.h | 84 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index acae122..0618b49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1164,9 +1164,7 @@ virKeycodeValueFromString; virKeycodeValueTranslate; # xml.h -virXMLParseFileHelper; virXMLParseHelper; -virXMLParseStrHelper; virXMLPropString; virXPathBoolean; virXPathInt; diff --git a/src/util/xml.c b/src/util/xml.c index 05317ea..d301af6 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -662,6 +662,7 @@ catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) * @filename: file to be parsed or NULL if string parsing is requested * @xmlStr: XML string to be parsed in case filename is NULL * @url: URL of XML document for string parser + * @ctxt: optional pointer to populate with new context pointer * * Parse XML document provided either as a file or a string. The function * guarantees that the XML document contains a root element. @@ -672,7 +673,8 @@ xmlDocPtr virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, - const char *url) + const char *url, + xmlXPathContextPtr *ctxt) { struct virParserData private; xmlParserCtxtPtr pctxt; @@ -680,8 +682,10 @@ virXMLParseHelper(int domcode, /* Set up a parser context so we can catch the details of XML errors. */ pctxt = xmlNewParserCtxt(); - if (!pctxt || !pctxt->sax) + if (!pctxt || !pctxt->sax) { + virReportOOMError(); goto error; + } private.domcode = domcode; pctxt->_private = &private; @@ -705,6 +709,15 @@ virXMLParseHelper(int domcode, goto error; } + if (ctxt) { + *ctxt = xmlXPathNewContext(xml); + if (!*ctxt) { + virReportOOMError(); + goto error; + } + (*ctxt)->node = xmlDocGetRootElement(xml); + } + cleanup: xmlFreeParserCtxt(pctxt); @@ -720,39 +733,3 @@ error: } goto cleanup; } - -/** - * virXMLParseStrHelper: - * @domcode: error domain of the caller, usually VIR_FROM_THIS - * @xmlStr: XML string to be parsed in case filename is NULL - * @url: URL of XML document for string parser - * - * Parse XML document provided as a string. The function guarantees that - * the XML document contains a root element. - * - * Returns parsed XML document. - */ -xmlDocPtr -virXMLParseStrHelper(int domcode, - const char *xmlStr, - const char *url) -{ - return virXMLParseHelper(domcode, NULL, xmlStr, url); -} - -/** - * virXMLParseFileHelper: - * @domcode: error domain of the caller, usually VIR_FROM_THIS - * @filename: file to be parsed - * - * Parse XML document provided as a file. The function guarantees that - * the XML document contains a root element. - * - * Returns parsed XML document. - */ -xmlDocPtr -virXMLParseFileHelper(int domcode, - const char *filename) -{ - return virXMLParseHelper(domcode, filename, NULL, NULL); -} diff --git a/src/util/xml.h b/src/util/xml.h index b342e83..d30e066 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -53,23 +53,89 @@ int virXPathNodeSet(const char *xpath, char * virXMLPropString(xmlNodePtr node, const char *name); +/* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, - const char *url); -xmlDocPtr virXMLParseStrHelper(int domcode, - const char *xmlStr, - const char *url); -xmlDocPtr virXMLParseFileHelper(int domcode, - const char *filename); + const char *url, + xmlXPathContextPtr *pctxt); +/** + * virXMLParse: + * @filename: file to parse, or NULL for string parsing + * @xmlStr: if @filename is NULL, a string to parse + * @url: if @filename is NULL, an optional filename to attribute the parse to + * + * Parse xml from either a file or a string. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParse(filename, xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url) + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL) +/** + * virXMLParseString: + * @xmlStr: a string to parse + * @url: an optional filename to attribute the parse to + * + * Parse xml from a string. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParseString(xmlStr, url) \ - virXMLParseStrHelper(VIR_FROM_THIS, xmlStr, url) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL) +/** + * virXMLParseFile: + * @filename: file to parse + * + * Parse xml from a file. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParseFile(filename) \ - virXMLParseFileHelper(VIR_FROM_THIS, filename) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL) + +/** + * virXMLParseCtxt: + * @filename: file to parse, or NULL for string parsing + * @xmlStr: if @filename is NULL, a string to parse + * @url: if @filename is NULL, an optional filename to attribute the parse to + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from either a file or a string. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseCtxt(filename, xmlStr, url, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, pctxt) + +/** + * virXMLParseStringCtxt: + * @xmlStr: a string to parse + * @url: an optional filename to attribute the parse to + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from a string. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseStringCtxt(xmlStr, url, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, pctxt) + +/** + * virXMLParseFileCtxt: + * @filename: file to parse + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from a file. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseFileCtxt(filename, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt) #endif /* __VIR_XML_H__ */ -- 1.7.4.4
On Thu, Aug 18, 2011 at 03:40:48PM -0600, Eric Blake wrote:
Often, we want to use XPath functions on the just-parsed document; fold this into the parser function for convenience.
* src/util/xml.h (virXMLParseHelper): Add argument. (virXMLParseStrHelper, virXMLParseFileHelper): Delete. (virXMLParseCtxt, virXMLParseStringCtxt, virXMLParseFileCtxt): New macros. * src/libvirt_private.syms (xml.h): Remove deleted functions. * src/util/xml.c (virXMLParseHelper): Add argument. (virXMLParseStrHelper, virXMLParseFileHelper): Delete. --- src/libvirt_private.syms | 2 - src/util/xml.c | 53 ++++++++--------------------- src/util/xml.h | 84 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 90 insertions(+), 49 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index acae122..0618b49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1164,9 +1164,7 @@ virKeycodeValueFromString; virKeycodeValueTranslate;
# xml.h -virXMLParseFileHelper; virXMLParseHelper; -virXMLParseStrHelper; virXMLPropString; virXPathBoolean; virXPathInt; diff --git a/src/util/xml.c b/src/util/xml.c index 05317ea..d301af6 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -662,6 +662,7 @@ catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) * @filename: file to be parsed or NULL if string parsing is requested * @xmlStr: XML string to be parsed in case filename is NULL * @url: URL of XML document for string parser + * @ctxt: optional pointer to populate with new context pointer * * Parse XML document provided either as a file or a string. The function * guarantees that the XML document contains a root element. @@ -672,7 +673,8 @@ xmlDocPtr virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, - const char *url) + const char *url, + xmlXPathContextPtr *ctxt) { struct virParserData private; xmlParserCtxtPtr pctxt; @@ -680,8 +682,10 @@ virXMLParseHelper(int domcode,
/* Set up a parser context so we can catch the details of XML errors. */ pctxt = xmlNewParserCtxt(); - if (!pctxt || !pctxt->sax) + if (!pctxt || !pctxt->sax) { + virReportOOMError(); goto error; + }
private.domcode = domcode; pctxt->_private = &private; @@ -705,6 +709,15 @@ virXMLParseHelper(int domcode, goto error; }
+ if (ctxt) { + *ctxt = xmlXPathNewContext(xml); + if (!*ctxt) { + virReportOOMError(); + goto error; + } + (*ctxt)->node = xmlDocGetRootElement(xml); + } + cleanup: xmlFreeParserCtxt(pctxt);
@@ -720,39 +733,3 @@ error: } goto cleanup; } - -/** - * virXMLParseStrHelper: - * @domcode: error domain of the caller, usually VIR_FROM_THIS - * @xmlStr: XML string to be parsed in case filename is NULL - * @url: URL of XML document for string parser - * - * Parse XML document provided as a string. The function guarantees that - * the XML document contains a root element. - * - * Returns parsed XML document. - */ -xmlDocPtr -virXMLParseStrHelper(int domcode, - const char *xmlStr, - const char *url) -{ - return virXMLParseHelper(domcode, NULL, xmlStr, url); -} - -/** - * virXMLParseFileHelper: - * @domcode: error domain of the caller, usually VIR_FROM_THIS - * @filename: file to be parsed - * - * Parse XML document provided as a file. The function guarantees that - * the XML document contains a root element. - * - * Returns parsed XML document. - */ -xmlDocPtr -virXMLParseFileHelper(int domcode, - const char *filename) -{ - return virXMLParseHelper(domcode, filename, NULL, NULL); -} diff --git a/src/util/xml.h b/src/util/xml.h index b342e83..d30e066 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -53,23 +53,89 @@ int virXPathNodeSet(const char *xpath, char * virXMLPropString(xmlNodePtr node, const char *name);
+/* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, - const char *url); -xmlDocPtr virXMLParseStrHelper(int domcode, - const char *xmlStr, - const char *url); -xmlDocPtr virXMLParseFileHelper(int domcode, - const char *filename); + const char *url, + xmlXPathContextPtr *pctxt);
+/** + * virXMLParse: + * @filename: file to parse, or NULL for string parsing + * @xmlStr: if @filename is NULL, a string to parse + * @url: if @filename is NULL, an optional filename to attribute the parse to + * + * Parse xml from either a file or a string. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParse(filename, xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url) + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL)
+/** + * virXMLParseString: + * @xmlStr: a string to parse + * @url: an optional filename to attribute the parse to + * + * Parse xml from a string. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParseString(xmlStr, url) \ - virXMLParseStrHelper(VIR_FROM_THIS, xmlStr, url) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL)
+/** + * virXMLParseFile: + * @filename: file to parse + * + * Parse xml from a file. + * + * Return the parsed document object, or NULL on failure. + */ # define virXMLParseFile(filename) \ - virXMLParseFileHelper(VIR_FROM_THIS, filename) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL) + +/** + * virXMLParseCtxt: + * @filename: file to parse, or NULL for string parsing + * @xmlStr: if @filename is NULL, a string to parse + * @url: if @filename is NULL, an optional filename to attribute the parse to + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from either a file or a string. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseCtxt(filename, xmlStr, url, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, pctxt) + +/** + * virXMLParseStringCtxt: + * @xmlStr: a string to parse + * @url: an optional filename to attribute the parse to + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from a string. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseStringCtxt(xmlStr, url, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, pctxt) + +/** + * virXMLParseFileCtxt: + * @filename: file to parse + * @pctxt: if non-NULL, populate with a new context object on success, + * with (*pctxt)->node pre-set to the root node + * + * Parse xml from a file. + * + * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseFileCtxt(filename, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt)
#endif /* __VIR_XML_H__ */
ACK to the overall cleanup, I'm just a bit dubious about xmlXPathContextPtr initialization in the helper, as I like to have allocations and frees matched to the same level. It's easier to forget to free the context if one doesn't "see" it allocated. But it's a minor concern. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 08/19/2011 02:48 AM, Daniel Veillard wrote:
+ * Return the parsed document object, or NULL on failure. + */ +# define virXMLParseFileCtxt(filename, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt)
#endif /* __VIR_XML_H__ */
ACK to the overall cleanup, I'm just a bit dubious about xmlXPathContextPtr initialization in the helper, as I like to have allocations and frees matched to the same level. It's easier to forget to free the context if one doesn't "see" it allocated. But it's a minor concern.
I think you can still "see" the allocations - it's just that virXMLParse does one allocation (return value), while virXMLParseCtxt does two allocations (return value, and allocation into passed pointer). Yes, it does mean that one virXMLParseCtxt call results in two separate free calls, but hopefully we don't introduce leaks because of it, and coding by copy-paste should still work at cleaning both variables. At any rate, I've now pushed the series. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Repetitive patterns should be factored. The sign of a good factorization is a change that kills 5x more lines than it adds :) * src/conf/domain_conf.c (virDomainDeviceDefParse) (virDomainSnapshotDefParseString): Use new convenience macros. * src/conf/storage_conf.c (virStoragePoolDefParseSourceString): Likewise. * src/cpu/cpu.c (cpuCompareXML, cpuBaselineXML): Likewise. * src/esx/esx_vi.c (esxVI_Context_Execute): Likewise. * src/qemu/qemu_migration.c (qemuMigrationCookieXMLParseStr): Likewise. * src/security/virt-aa-helper.c (caps_mockup): Likewise. * src/test/test_driver.c (testOpenFromFile): Likewise. * tests/cputest.c (cpuTestLoadXML, cpuTestLoadMultiXML): Likewise. * tools/virsh.c (cmdFreecell, makeCloneXML, cmdVNCDisplay) (cmdTTYConsole, cmdDetachInterface, cmdDetachDisk) (cmdSnapshotCreate, cmdSnapshotCreateAs, cmdSnapshotCurrent) (cmdSnapshotList, cmdSnapshotParent): Likewise. --- src/conf/domain_conf.c | 20 +-------- src/conf/storage_conf.c | 8 +--- src/cpu/cpu.c | 18 +------- src/esx/esx_vi.c | 13 +----- src/qemu/qemu_migration.c | 9 +---- src/security/virt-aa-helper.c | 12 +----- src/test/test_driver.c | 13 +----- tests/cputest.c | 9 +--- tools/virsh.c | 84 ++++++---------------------------------- 9 files changed, 30 insertions(+), 156 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce1f3c5..0a2c9eb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5385,17 +5385,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; - if (!(xml = virXMLParseString(xmlStr, "device.xml"))) { + if (!(xml = virXMLParseStringCtxt(xmlStr, "device.xml", &ctxt))) { goto error; } - node = xmlDocGetRootElement(xml); - - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); - goto error; - } - ctxt->node = xmlDocGetRootElement(xml); + node = ctxt->node; if (VIR_ALLOC(dev) < 0) { virReportOOMError(); @@ -10968,23 +10961,16 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, char *creation = NULL, *state = NULL; struct timeval tv; - xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml"); + xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt); if (!xml) { return NULL; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); - goto cleanup; - } - if (VIR_ALLOC(def) < 0) { virReportOOMError(); goto cleanup; } - ctxt->node = xmlDocGetRootElement(xml); if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domainsnapshot")) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); goto cleanup; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..8d14e87 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -505,13 +505,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec, xmlXPathContextPtr xpath_ctxt = NULL; virStoragePoolSourcePtr def = NULL, ret = NULL; - if (!(doc = virXMLParseString(srcSpec, "srcSpec.xml"))) { - goto cleanup; - } - - xpath_ctxt = xmlXPathNewContext(doc); - if (xpath_ctxt == NULL) { - virReportOOMError(); + if (!(doc = virXMLParseStringCtxt(srcSpec, "srcSpec.xml", &xpath_ctxt))) { goto cleanup; } diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 16e0709..b47f078 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -1,7 +1,7 @@ /* * cpu.c: internal functions for CPU manipulation * - * Copyright (C) 2009--2010 Red Hat, Inc. + * Copyright (C) 2009-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -76,16 +76,9 @@ cpuCompareXML(virCPUDefPtr host, VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml)); - if (!(doc = virXMLParseString(xml, "cpu.xml"))) + if (!(doc = virXMLParseStringCtxt(xml, "cpu.xml", &ctxt))) goto cleanup; - if ((ctxt = xmlXPathNewContext(doc)) == NULL) { - virReportOOMError(); - goto cleanup; - } - - ctxt->node = xmlDocGetRootElement(doc); - cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO); if (cpu == NULL) goto cleanup; @@ -311,14 +304,9 @@ cpuBaselineXML(const char **xmlCPUs, goto no_memory; for (i = 0; i < ncpus; i++) { - if (!(doc = virXMLParseString(xmlCPUs[i], "cpu.xml"))) + if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], "cpu.xml", &ctxt))) goto error; - if ((ctxt = xmlXPathNewContext(doc)) == NULL) - goto no_memory; - - ctxt->node = xmlDocGetRootElement(doc); - cpus[i] = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_HOST); if (cpus[i] == NULL) goto error; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c82094b..5c8d79e 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -910,21 +910,14 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, (*response)->content = virBufferContentAndReset(&buffer); if ((*response)->responseCode == 500 || (*response)->responseCode == 200) { - (*response)->document = virXMLParseString((*response)->content, - "esx.xml"); + (*response)->document = virXMLParseStringCtxt((*response)->content, + "esx.xml", + &xpathContext); if ((*response)->document == NULL) { goto cleanup; } - xpathContext = xmlXPathNewContext((*response)->document); - - if (xpathContext == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not create XPath context")); - goto cleanup; - } - xmlXPathRegisterNs(xpathContext, BAD_CAST "soapenv", BAD_CAST "http://schemas.xmlsoap.org/soap/envelope/"); xmlXPathRegisterNs(xpathContext, BAD_CAST "vim", BAD_CAST "urn:vim25"); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5510493..a84faf6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -602,16 +602,9 @@ qemuMigrationCookieXMLParseStr(qemuMigrationCookiePtr mig, VIR_DEBUG("xml=%s", NULLSTR(xml)); - if (!(doc = virXMLParseString(xml, "qemumigration.xml"))) + if (!(doc = virXMLParseStringCtxt(xml, "qemumigration.xml", &ctxt))) goto cleanup; - if ((ctxt = xmlXPathNewContext(doc)) == NULL) { - virReportOOMError(); - goto cleanup; - } - - ctxt->node = xmlDocGetRootElement(doc); - ret = qemuMigrationCookieXMLParse(mig, ctxt, flags); cleanup: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 1e2feae..bb577d3 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -640,24 +640,16 @@ caps_mockup(vahControl * ctl, const char *xmlStr) int rc = -1; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; - xmlNodePtr root; - if (!(xml = virXMLParseString(xmlStr, "domain.xml"))) { + if (!(xml = virXMLParseStringCtxt(xmlStr, "domain.xml", &ctxt))) { goto cleanup; } - root = xmlDocGetRootElement(xml); - if (!xmlStrEqual(root->name, BAD_CAST "domain")) { + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domain")) { vah_error(NULL, 0, _("incorrect root element")); goto cleanup; } - if ((ctxt = xmlXPathNewContext(xml)) == NULL) { - vah_error(ctl, 0, _("could not allocate memory")); - goto cleanup; - } - ctxt->node = root; - /* Quick sanity check for some required elements */ if (verify_xpath_context(ctxt) != 0) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fb14b10..544fd7e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -742,7 +742,6 @@ static int testOpenFromFile(virConnectPtr conn, long l; char *str; xmlDocPtr xml = NULL; - xmlNodePtr root = NULL; xmlNodePtr *domains = NULL, *networks = NULL, *ifaces = NULL, *pools = NULL, *devs = NULL; xmlXPathContextPtr ctxt = NULL; @@ -771,24 +770,16 @@ static int testOpenFromFile(virConnectPtr conn, if (!(privconn->caps = testBuildCapabilities(conn))) goto error; - if (!(xml = virXMLParseFile(file))) { + if (!(xml = virXMLParseFileCtxt(file, &ctxt))) { goto error; } - root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) { + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) { testError(VIR_ERR_XML_ERROR, "%s", _("Root element is not 'node'")); goto error; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - testError(VIR_ERR_INTERNAL_ERROR, "%s", - _("creating xpath context")); - goto error; - } - privconn->nextDomID = 1; privconn->numCells = 0; if ((privconn->path = strdup(file)) == NULL) { diff --git a/tests/cputest.c b/tests/cputest.c index 4dcf1aa..b5272c1 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -96,11 +96,9 @@ cpuTestLoadXML(const char *arch, const char *name) if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) goto cleanup; - if (!(doc = virXMLParseFile(xml)) || - !(ctxt = xmlXPathNewContext(doc))) + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) goto cleanup; - ctxt->node = xmlDocGetRootElement(doc); cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO); cleanup: @@ -127,12 +125,9 @@ cpuTestLoadMultiXML(const char *arch, if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) goto cleanup; - if (!(doc = virXMLParseFile(xml)) || - !(ctxt = xmlXPathNewContext(doc))) + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) goto error; - ctxt->node = xmlDocGetRootElement(doc); - n = virXPathNodeSet("/cpuTest/cpu", ctxt, &nodes); if (n <= 0 || !(cpus = calloc(n, sizeof(virCPUDefPtr)))) goto error; diff --git a/tools/virsh.c b/tools/virsh.c index c43de4c..1c67150 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -67,6 +67,8 @@ static char *progname; #define VSH_PROMPT_RW "virsh # " #define VSH_PROMPT_RO "virsh > " +#define VIR_FROM_THIS VIR_FROM_NONE + #define GETTIMEOFDAY(T) gettimeofday(T, NULL) #define DIFF_MSEC(T, U) \ ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \ @@ -2901,16 +2903,11 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - xml = xmlReadDoc((const xmlChar *) cap_xml, "node.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - + xml = virXMLParseStringCtxt(cap_xml, "node.xml", &ctxt); if (!xml) { vshError(ctl, "%s", _("unable to get node capabilities")); goto cleanup; } - - ctxt = xmlXPathNewContext(xml); nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell", ctxt, &nodes); @@ -8404,13 +8401,9 @@ makeCloneXML(const char *origxml, const char *newname) { xmlChar *newxml = NULL; int size; - doc = xmlReadDoc((const xmlChar *) origxml, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOWARNING); + doc = virXMLParseStringCtxt(origxml, "domain.xml", &ctxt); if (!doc) goto cleanup; - ctxt = xmlXPathNewContext(doc); - if (!ctxt) - goto cleanup; obj = xmlXPathEval(BAD_CAST "/volume/name", ctxt); if ((obj == NULL) || (obj->nodesetval == NULL) || @@ -10199,15 +10192,10 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup; obj = xmlXPathEval(BAD_CAST "string(/domain/devices/graphics[@type='vnc']/@port)", ctxt); if ((obj == NULL) || (obj->type != XPATH_STRING) || @@ -10272,15 +10260,10 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup; obj = xmlXPathEval(BAD_CAST "string(/domain/devices/console/@tty)", ctxt); if ((obj == NULL) || (obj->type != XPATH_STRING) || @@ -10664,19 +10647,12 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) { vshError(ctl, "%s", _("Failed to get interface information")); goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (!ctxt) { - vshError(ctl, "%s", _("Failed to get interface information")); - goto cleanup; - } snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type); obj = xmlXPathEval(BAD_CAST buf, ctxt); @@ -11138,19 +11114,12 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) { vshError(ctl, "%s", _("Failed to get disk information")); goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (!ctxt) { - vshError(ctl, "%s", _("Failed to get disk information")); - goto cleanup; - } obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt); if ((obj == NULL) || (obj->type != XPATH_NODESET) || @@ -11865,14 +11834,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup; name = virXPathString("string(/domainsnapshot/name)", ctxt); if (!name) { @@ -11974,14 +11938,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup; - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup; parsed_name = virXPathString("string(/domainsnapshot/name)", ctxt); if (!parsed_name) { @@ -12056,16 +12015,9 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) xmlDocPtr xmldoc = NULL; xmlXPathContextPtr ctxt = NULL; - xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", - NULL, XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt); if (!xmldoc) goto cleanup; - ctxt = xmlXPathNewContext(xmldoc); - if (!ctxt) { - xmlFreeDoc(xmldoc); - goto cleanup; - } name = virXPathString("string(/domainsnapshot/name)", ctxt); xmlXPathFreeContext(ctxt); @@ -12165,14 +12117,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (!doc) continue; - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) continue; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - continue; state = virXPathString("string(/domainsnapshot/state)", ctxt); if (state == NULL) @@ -12312,14 +12259,9 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (!xml) goto cleanup; - xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt); if (!xmldoc) goto cleanup; - ctxt = xmlXPathNewContext(xmldoc); - if (!ctxt) - goto cleanup; parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt); if (!parent) -- 1.7.4.4
On Thu, Aug 18, 2011 at 03:40:49PM -0600, Eric Blake wrote:
Repetitive patterns should be factored. The sign of a good factorization is a change that kills 5x more lines than it adds :)
* src/conf/domain_conf.c (virDomainDeviceDefParse) (virDomainSnapshotDefParseString): Use new convenience macros. * src/conf/storage_conf.c (virStoragePoolDefParseSourceString): Likewise. * src/cpu/cpu.c (cpuCompareXML, cpuBaselineXML): Likewise. * src/esx/esx_vi.c (esxVI_Context_Execute): Likewise. * src/qemu/qemu_migration.c (qemuMigrationCookieXMLParseStr): Likewise. * src/security/virt-aa-helper.c (caps_mockup): Likewise. * src/test/test_driver.c (testOpenFromFile): Likewise. * tests/cputest.c (cpuTestLoadXML, cpuTestLoadMultiXML): Likewise. * tools/virsh.c (cmdFreecell, makeCloneXML, cmdVNCDisplay) (cmdTTYConsole, cmdDetachInterface, cmdDetachDisk) (cmdSnapshotCreate, cmdSnapshotCreateAs, cmdSnapshotCurrent) (cmdSnapshotList, cmdSnapshotParent): Likewise. --- src/conf/domain_conf.c | 20 +-------- src/conf/storage_conf.c | 8 +--- src/cpu/cpu.c | 18 +------- src/esx/esx_vi.c | 13 +----- src/qemu/qemu_migration.c | 9 +---- src/security/virt-aa-helper.c | 12 +----- src/test/test_driver.c | 13 +----- tests/cputest.c | 9 +--- tools/virsh.c | 84 ++++++---------------------------------- 9 files changed, 30 insertions(+), 156 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce1f3c5..0a2c9eb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5385,17 +5385,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL;
- if (!(xml = virXMLParseString(xmlStr, "device.xml"))) { + if (!(xml = virXMLParseStringCtxt(xmlStr, "device.xml", &ctxt))) { goto error; } - node = xmlDocGetRootElement(xml); - - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); - goto error; - } - ctxt->node = xmlDocGetRootElement(xml); + node = ctxt->node;
if (VIR_ALLOC(dev) < 0) { virReportOOMError(); @@ -10968,23 +10961,16 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, char *creation = NULL, *state = NULL; struct timeval tv;
- xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml"); + xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt); if (!xml) { return NULL; }
- ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); - goto cleanup; - } - if (VIR_ALLOC(def) < 0) { virReportOOMError(); goto cleanup; }
- ctxt->node = xmlDocGetRootElement(xml); if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domainsnapshot")) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); goto cleanup; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 995f9a6..8d14e87 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -505,13 +505,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec, xmlXPathContextPtr xpath_ctxt = NULL; virStoragePoolSourcePtr def = NULL, ret = NULL;
- if (!(doc = virXMLParseString(srcSpec, "srcSpec.xml"))) { - goto cleanup; - } - - xpath_ctxt = xmlXPathNewContext(doc); - if (xpath_ctxt == NULL) { - virReportOOMError(); + if (!(doc = virXMLParseStringCtxt(srcSpec, "srcSpec.xml", &xpath_ctxt))) { goto cleanup; }
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 16e0709..b47f078 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -1,7 +1,7 @@ /* * cpu.c: internal functions for CPU manipulation * - * Copyright (C) 2009--2010 Red Hat, Inc. + * Copyright (C) 2009-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -76,16 +76,9 @@ cpuCompareXML(virCPUDefPtr host,
VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml));
- if (!(doc = virXMLParseString(xml, "cpu.xml"))) + if (!(doc = virXMLParseStringCtxt(xml, "cpu.xml", &ctxt))) goto cleanup;
- if ((ctxt = xmlXPathNewContext(doc)) == NULL) { - virReportOOMError(); - goto cleanup; - } - - ctxt->node = xmlDocGetRootElement(doc); - cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO); if (cpu == NULL) goto cleanup; @@ -311,14 +304,9 @@ cpuBaselineXML(const char **xmlCPUs, goto no_memory;
for (i = 0; i < ncpus; i++) { - if (!(doc = virXMLParseString(xmlCPUs[i], "cpu.xml"))) + if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], "cpu.xml", &ctxt))) goto error;
- if ((ctxt = xmlXPathNewContext(doc)) == NULL) - goto no_memory; - - ctxt->node = xmlDocGetRootElement(doc); - cpus[i] = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_HOST); if (cpus[i] == NULL) goto error; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c82094b..5c8d79e 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -910,21 +910,14 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, (*response)->content = virBufferContentAndReset(&buffer);
if ((*response)->responseCode == 500 || (*response)->responseCode == 200) { - (*response)->document = virXMLParseString((*response)->content, - "esx.xml"); + (*response)->document = virXMLParseStringCtxt((*response)->content, + "esx.xml", + &xpathContext);
if ((*response)->document == NULL) { goto cleanup; }
- xpathContext = xmlXPathNewContext((*response)->document); - - if (xpathContext == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not create XPath context")); - goto cleanup; - } - xmlXPathRegisterNs(xpathContext, BAD_CAST "soapenv", BAD_CAST "http://schemas.xmlsoap.org/soap/envelope/"); xmlXPathRegisterNs(xpathContext, BAD_CAST "vim", BAD_CAST "urn:vim25"); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5510493..a84faf6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -602,16 +602,9 @@ qemuMigrationCookieXMLParseStr(qemuMigrationCookiePtr mig,
VIR_DEBUG("xml=%s", NULLSTR(xml));
- if (!(doc = virXMLParseString(xml, "qemumigration.xml"))) + if (!(doc = virXMLParseStringCtxt(xml, "qemumigration.xml", &ctxt))) goto cleanup;
- if ((ctxt = xmlXPathNewContext(doc)) == NULL) { - virReportOOMError(); - goto cleanup; - } - - ctxt->node = xmlDocGetRootElement(doc); - ret = qemuMigrationCookieXMLParse(mig, ctxt, flags);
cleanup: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 1e2feae..bb577d3 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -640,24 +640,16 @@ caps_mockup(vahControl * ctl, const char *xmlStr) int rc = -1; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; - xmlNodePtr root;
- if (!(xml = virXMLParseString(xmlStr, "domain.xml"))) { + if (!(xml = virXMLParseStringCtxt(xmlStr, "domain.xml", &ctxt))) { goto cleanup; }
- root = xmlDocGetRootElement(xml); - if (!xmlStrEqual(root->name, BAD_CAST "domain")) { + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domain")) { vah_error(NULL, 0, _("incorrect root element")); goto cleanup; }
- if ((ctxt = xmlXPathNewContext(xml)) == NULL) { - vah_error(ctl, 0, _("could not allocate memory")); - goto cleanup; - } - ctxt->node = root; - /* Quick sanity check for some required elements */ if (verify_xpath_context(ctxt) != 0) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fb14b10..544fd7e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -742,7 +742,6 @@ static int testOpenFromFile(virConnectPtr conn, long l; char *str; xmlDocPtr xml = NULL; - xmlNodePtr root = NULL; xmlNodePtr *domains = NULL, *networks = NULL, *ifaces = NULL, *pools = NULL, *devs = NULL; xmlXPathContextPtr ctxt = NULL; @@ -771,24 +770,16 @@ static int testOpenFromFile(virConnectPtr conn, if (!(privconn->caps = testBuildCapabilities(conn))) goto error;
- if (!(xml = virXMLParseFile(file))) { + if (!(xml = virXMLParseFileCtxt(file, &ctxt))) { goto error; }
- root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) { + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) { testError(VIR_ERR_XML_ERROR, "%s", _("Root element is not 'node'")); goto error; }
- ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - testError(VIR_ERR_INTERNAL_ERROR, "%s", - _("creating xpath context")); - goto error; - } - privconn->nextDomID = 1; privconn->numCells = 0; if ((privconn->path = strdup(file)) == NULL) { diff --git a/tests/cputest.c b/tests/cputest.c index 4dcf1aa..b5272c1 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -96,11 +96,9 @@ cpuTestLoadXML(const char *arch, const char *name) if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) goto cleanup;
- if (!(doc = virXMLParseFile(xml)) || - !(ctxt = xmlXPathNewContext(doc))) + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) goto cleanup;
- ctxt->node = xmlDocGetRootElement(doc); cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);
cleanup: @@ -127,12 +125,9 @@ cpuTestLoadMultiXML(const char *arch, if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) goto cleanup;
- if (!(doc = virXMLParseFile(xml)) || - !(ctxt = xmlXPathNewContext(doc))) + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) goto error;
- ctxt->node = xmlDocGetRootElement(doc); - n = virXPathNodeSet("/cpuTest/cpu", ctxt, &nodes); if (n <= 0 || !(cpus = calloc(n, sizeof(virCPUDefPtr)))) goto error; diff --git a/tools/virsh.c b/tools/virsh.c index c43de4c..1c67150 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -67,6 +67,8 @@ static char *progname; #define VSH_PROMPT_RW "virsh # " #define VSH_PROMPT_RO "virsh > "
+#define VIR_FROM_THIS VIR_FROM_NONE + #define GETTIMEOFDAY(T) gettimeofday(T, NULL) #define DIFF_MSEC(T, U) \ ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \ @@ -2901,16 +2903,11 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) goto cleanup; }
- xml = xmlReadDoc((const xmlChar *) cap_xml, "node.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - + xml = virXMLParseStringCtxt(cap_xml, "node.xml", &ctxt); if (!xml) { vshError(ctl, "%s", _("unable to get node capabilities")); goto cleanup; } - - ctxt = xmlXPathNewContext(xml); nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell", ctxt, &nodes);
@@ -8404,13 +8401,9 @@ makeCloneXML(const char *origxml, const char *newname) { xmlChar *newxml = NULL; int size;
- doc = xmlReadDoc((const xmlChar *) origxml, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOWARNING); + doc = virXMLParseStringCtxt(origxml, "domain.xml", &ctxt); if (!doc) goto cleanup; - ctxt = xmlXPathNewContext(doc); - if (!ctxt) - goto cleanup;
obj = xmlXPathEval(BAD_CAST "/volume/name", ctxt); if ((obj == NULL) || (obj->nodesetval == NULL) || @@ -10199,15 +10192,10 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup;
obj = xmlXPathEval(BAD_CAST "string(/domain/devices/graphics[@type='vnc']/@port)", ctxt); if ((obj == NULL) || (obj->type != XPATH_STRING) || @@ -10272,15 +10260,10 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup;
obj = xmlXPathEval(BAD_CAST "string(/domain/devices/console/@tty)", ctxt); if ((obj == NULL) || (obj->type != XPATH_STRING) || @@ -10664,19 +10647,12 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) { vshError(ctl, "%s", _("Failed to get interface information")); goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (!ctxt) { - vshError(ctl, "%s", _("Failed to get interface information")); - goto cleanup; - }
snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type); obj = xmlXPathEval(BAD_CAST buf, ctxt); @@ -11138,19 +11114,12 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt); VIR_FREE(doc); if (!xml) { vshError(ctl, "%s", _("Failed to get disk information")); goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (!ctxt) { - vshError(ctl, "%s", _("Failed to get disk information")); - goto cleanup; - }
obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt); if ((obj == NULL) || (obj->type != XPATH_NODESET) || @@ -11865,14 +11834,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup;
name = virXPathString("string(/domainsnapshot/name)", ctxt); if (!name) { @@ -11974,14 +11938,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (!doc) goto cleanup;
- xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) goto cleanup; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - goto cleanup;
parsed_name = virXPathString("string(/domainsnapshot/name)", ctxt); if (!parsed_name) { @@ -12056,16 +12015,9 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) xmlDocPtr xmldoc = NULL; xmlXPathContextPtr ctxt = NULL;
- xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", - NULL, XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt); if (!xmldoc) goto cleanup; - ctxt = xmlXPathNewContext(xmldoc); - if (!ctxt) { - xmlFreeDoc(xmldoc); - goto cleanup; - }
name = virXPathString("string(/domainsnapshot/name)", ctxt); xmlXPathFreeContext(ctxt); @@ -12165,14 +12117,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (!doc) continue;
- xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); if (!xml) continue; - ctxt = xmlXPathNewContext(xml); - if (!ctxt) - continue;
state = virXPathString("string(/domainsnapshot/state)", ctxt); if (state == NULL) @@ -12312,14 +12259,9 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (!xml) goto cleanup;
- xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt); if (!xmldoc) goto cleanup; - ctxt = xmlXPathNewContext(xmldoc); - if (!ctxt) - goto cleanup;
parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt); if (!parent)
ACK, okay it does remove lines :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 08/18/2011 03:40 PM, Eric Blake wrote:
I noticed this while working on my snapshot stuff. There was lots of copy and paste going on in virsh.c, and it felt awful to not be able to represent the same idea in fewer lines, so I fixed it.
This will probably mean I have to rebase the entire snapshot series, since it will introduce several conflicts in at least virsh.c; oh well.
I got lucky (yay!) - no rebase conflicts until patch 31/26 (unposted as of yet). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Aug 18, 2011 at 03:46:01PM -0600, Eric Blake wrote:
On 08/18/2011 03:40 PM, Eric Blake wrote:
I noticed this while working on my snapshot stuff. There was lots of copy and paste going on in virsh.c, and it felt awful to not be able to represent the same idea in fewer lines, so I fixed it.
This will probably mean I have to rebase the entire snapshot series, since it will introduce several conflicts in at least virsh.c; oh well.
I got lucky (yay!) - no rebase conflicts until patch 31/26 (unposted as of yet).
please push :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard -
Eric Blake