[libvirt] [PATCH 0/3] Refactor XML parsing code
Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places. Jiri Denemark (3): Introduce XML parsing utility functions Use common XML parsing functions Fix error reporting when parsing CPU XML strings src/conf/domain_conf.c | 130 ++++++--------------------------------- src/conf/interface_conf.c | 93 ++++------------------------ src/conf/network_conf.c | 93 ++++------------------------ src/conf/node_device_conf.c | 60 +----------------- src/conf/secret_conf.c | 56 ++--------------- src/conf/storage_conf.c | 110 +++------------------------------- src/cpu/cpu.c | 11 ++- src/util/xml.c | 142 ++++++++++++++++++++++++++++++++++++++++++- src/util/xml.h | 19 ++++++ 9 files changed, 231 insertions(+), 483 deletions(-)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/xml.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/util/xml.h | 19 ++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) diff --git a/src/util/xml.c b/src/util/xml.c index 46ea9aa..65bc961 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -25,10 +25,19 @@ #define VIR_FROM_THIS VIR_FROM_XML -#define virXMLError(code, fmt...) \ - virReportErrorHelper(NULL, VIR_FROM_XML, code, __FILE__, \ +#define virGenericReportError(from, code, fmt...) \ + virReportErrorHelper(NULL, from, code, __FILE__, \ __FUNCTION__, __LINE__, fmt) +#define virXMLError(code, fmt...) \ + virGenericReportError(VIR_FROM_XML, code, fmt) + + +/* Internal data to be passed to SAX parser and used by error handler. */ +struct virParserData { + int domcode; +}; + /************************************************************************ * * @@ -501,3 +510,132 @@ virXPathNodeSet(const char *xpath, xmlXPathFreeObject(obj); return (ret); } + + +/** + * catchXMLError: + * + * Called from SAX on parsing errors in the XML. + */ +static void +catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) +{ + int domcode = VIR_FROM_XML; + xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; + + if (ctxt) { + if (ctxt->_private) + domcode = ((struct virParserData *) ctxt->_private)->domcode; + + if (virGetLastError() == NULL && + ctxt->lastError.level == XML_ERR_FATAL && + ctxt->lastError.message != NULL) { + virGenericReportError(domcode, VIR_ERR_XML_DETAIL, + _("at line %d: %s"), + ctxt->lastError.line, + ctxt->lastError.message); + } + } +} + + +/** + * virXMLParseHelper: + * @domcode: error domain of the caller, usually VIR_FROM_THIS + * @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 + * + * Parse XML document provided either as a file or a string. The function + * guarantees that the XML document contains a root element. + * + * Returns parsed XML document. + */ +xmlDocPtr +virXMLParseHelper(int domcode, + const char *filename, + const char *xmlStr, + const char *url) +{ + struct virParserData private; + xmlParserCtxtPtr pctxt; + xmlDocPtr xml = NULL; + + /* Set up a parser context so we can catch the details of XML errors. */ + pctxt = xmlNewParserCtxt(); + if (!pctxt || !pctxt->sax) + goto error; + + private.domcode = domcode; + pctxt->_private = &private; + pctxt->sax->error = catchXMLError; + + if (filename) { + xml = xmlCtxtReadFile(pctxt, filename, NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + } else { + xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + } + if (!xml) + goto error; + + if (xmlDocGetRootElement(xml) == NULL) { + virGenericReportError(domcode, VIR_ERR_INTERNAL_ERROR, + "%s", _("missing root element")); + goto error; + } + +cleanup: + xmlFreeParserCtxt(pctxt); + + return xml; + +error: + xmlFreeDoc(xml); + xml = NULL; + + if (virGetLastError() == NULL) { + virGenericReportError(domcode, VIR_ERR_XML_ERROR, + "%s", _("failed to parse xml document")); + } + 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 246672d..8137f90 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -44,4 +44,23 @@ int virXPathNodeSet(const char *xpath, char * virXMLPropString(xmlNodePtr node, const char *name); +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); + +#define virXMLParse(filename, xmlStr, url) \ + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url) + +#define virXMLParseString(xmlStr, url) \ + virXMLParseStrHelper(VIR_FROM_THIS, xmlStr, url) + +#define virXMLParseFile(filename) \ + virXMLParseFileHelper(VIR_FROM_THIS, filename) + #endif /* __VIR_XML_H__ */ -- 1.7.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 130 +++++++------------------------------------ src/conf/interface_conf.c | 93 ++++-------------------------- src/conf/network_conf.c | 93 ++++-------------------------- src/conf/node_device_conf.c | 60 +------------------ src/conf/secret_conf.c | 56 ++---------------- src/conf/storage_conf.c | 110 +++--------------------------------- 6 files changed, 65 insertions(+), 477 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4ffeb8a..cdd906e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4130,99 +4130,35 @@ error: } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) +static virDomainDefPtr +virDomainDefParse(const char *xmlStr, + const char *filename, + virCapsPtr caps, + int flags) { - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virDomainReportError(VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } + xmlDocPtr xml; + virDomainDefPtr def = NULL; + + if ((xml = virXMLParse(filename, xmlStr, "domain.xml"))) { + def = virDomainDefParseNode(caps, xml, xmlDocGetRootElement(xml), flags); + xmlFreeDoc(xml); } + + return def; } virDomainDefPtr virDomainDefParseString(virCapsPtr caps, const char *xmlStr, int flags) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; - virDomainDefPtr def = NULL; - - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virDomainDefParseNode(caps, xml, root, flags); - -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); - return def; + return virDomainDefParse(xmlStr, NULL, caps, flags); } virDomainDefPtr virDomainDefParseFile(virCapsPtr caps, - const char *filename, int flags) + const char *filename, + int flags) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; - virDomainDefPtr def = NULL; - - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virDomainDefParseNode(caps, xml, root, flags); - -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); - return def; + return virDomainDefParse(NULL, filename, caps, flags); } @@ -4258,38 +4194,14 @@ cleanup: virDomainObjPtr virDomainObjParseFile(virCapsPtr caps, const char *filename) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; + xmlDocPtr xml; virDomainObjPtr obj = NULL; - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; + if ((xml = virXMLParseFile(filename))) { + obj = virDomainObjParseNode(caps, xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - obj = virDomainObjParseNode(caps, xml, root); - -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); return obj; } diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..33875a8 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -853,96 +853,29 @@ cleanup: return def; } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) -{ - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virInterfaceReportError (VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } - } -} - -virInterfaceDefPtr virInterfaceDefParseString(const char *xmlStr) +static virInterfaceDefPtr +virInterfaceDefParse(const char *xmlStr, + const char *filename) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; + xmlDocPtr xml; virInterfaceDefPtr def = NULL; - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "interface.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virInterfaceReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; + if ((xml = virXMLParse(filename, xmlStr, "interface.xml"))) { + def = virInterfaceDefParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virInterfaceReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virInterfaceDefParseNode(xml, root); - -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); return def; } -virInterfaceDefPtr virInterfaceDefParseFile(const char *filename) +virInterfaceDefPtr virInterfaceDefParseString(const char *xmlStr) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; - virInterfaceDefPtr def = NULL; - - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virInterfaceReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virInterfaceReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virInterfaceDefParseNode(xml, root); + return virInterfaceDefParse(xmlStr, NULL); +} -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); - return def; +virInterfaceDefPtr virInterfaceDefParseFile(const char *filename) +{ + return virInterfaceDefParse(NULL, filename); } static int diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6d3c3c0..014d2b8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -504,96 +504,29 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) return NULL; } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) -{ - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } - } -} - -virNetworkDefPtr virNetworkDefParseString(const char *xmlStr) +static virNetworkDefPtr +virNetworkDefParse(const char *xmlStr, + const char *filename) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; + xmlDocPtr xml; virNetworkDefPtr def = NULL; - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "network.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virNetworkReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virNetworkReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; + if ((xml = virXMLParse(filename, xmlStr, "network.xml"))) { + def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - def = virNetworkDefParseNode(xml, root); - -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); return def; } -virNetworkDefPtr virNetworkDefParseFile(const char *filename) +virNetworkDefPtr virNetworkDefParseString(const char *xmlStr) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; - virNetworkDefPtr def = NULL; - - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - virNetworkReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virNetworkReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virNetworkDefParseNode(xml, root); + return virNetworkDefParse(xmlStr, NULL); +} -cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc (xml); - return def; +virNetworkDefPtr virNetworkDefParseFile(const char *filename) +{ + return virNetworkDefParse(NULL, filename); } diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 09c0f41..7f2dac8 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1224,71 +1224,19 @@ cleanup: return def; } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) -{ - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virNodeDeviceReportError(VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } - } -} - - - static virNodeDeviceDefPtr virNodeDeviceDefParse(const char *str, const char *filename, int create) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; + xmlDocPtr xml; virNodeDeviceDefPtr def = NULL; - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - if (filename) { - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } else { - xml = xmlCtxtReadDoc (pctxt, BAD_CAST str, - "device.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + if ((xml = virXMLParse(filename, str, "device.xml"))) { + def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), create); + xmlFreeDoc(xml); } - if (!xml) { - if (virGetLastError() == NULL) - virNodeDeviceReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - def = virNodeDeviceDefParseNode(xml, root, create); - -cleanup: - xmlFreeParserCtxt(pctxt); - xmlFreeDoc(xml); return def; } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 946d425..bbdad89 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -187,62 +187,18 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) return ret; } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) -{ - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virSecretReportError(VIR_ERR_XML_DETAIL, _("at line %d: %s"), - ctxt->lastError.line, ctxt->lastError.message); - } - } -} - static virSecretDefPtr -virSecretDefParse(const char *xmlStr, const char *filename) +virSecretDefParse(const char *xmlStr, + const char *filename) { - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr root; + xmlDocPtr xml; virSecretDefPtr ret = NULL; - pctxt = xmlNewParserCtxt(); - if (pctxt == NULL || pctxt->sax == NULL) - goto cleanup; - pctxt->sax->error = catchXMLError; - - if (filename != NULL) - xml = xmlCtxtReadFile(pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - else - xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, "secret.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (xml == NULL) { - if (virGetLastError() == NULL) - virSecretReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to parse xml document")); - goto cleanup; - } - - root = xmlDocGetRootElement(xml); - if (root == NULL) { - virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing root element")); - goto cleanup; + if ((xml = virXMLParse(filename, xmlStr, "secret.xml"))) { + ret = secretXMLParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - ret = secretXMLParseNode(xml, root); - - cleanup: - xmlFreeDoc(xml); - xmlFreeParserCtxt(pctxt); return ret; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 19a1db9..bc60160 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -708,24 +708,6 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { return NULL; } -/* Called from SAX on parsing errors in the XML. */ -static void -catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) -{ - xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx; - - if (ctxt) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - virStorageReportError (VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } - } -} - virStoragePoolDefPtr virStoragePoolDefParseNode(xmlDocPtr xml, xmlNodePtr root) { @@ -755,52 +737,14 @@ static virStoragePoolDefPtr virStoragePoolDefParse(const char *xmlStr, const char *filename) { virStoragePoolDefPtr ret = NULL; - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr node = NULL; + xmlDocPtr xml; - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - if (filename) { - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } else { - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, - "storage.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); + if ((xml = virXMLParse(filename, xmlStr, "storage.xml"))) { + ret = virStoragePoolDefParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - if (!xml) { - if (virGetLastError() == NULL) - virStorageReportError(VIR_ERR_XML_ERROR, - "%s",_("failed to parse xml document")); - goto cleanup; - } - - node = xmlDocGetRootElement(xml); - if (node == NULL) { - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - ret = virStoragePoolDefParseNode(xml, node); - - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - return ret; - - cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - return NULL; } virStoragePoolDefPtr @@ -1154,52 +1098,14 @@ virStorageVolDefParse(virStoragePoolDefPtr pool, const char *xmlStr, const char *filename) { virStorageVolDefPtr ret = NULL; - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr node = NULL; - - /* Set up a parser context so we can catch the details of XML errors. */ - pctxt = xmlNewParserCtxt (); - if (!pctxt || !pctxt->sax) - goto cleanup; - pctxt->sax->error = catchXMLError; - - if (filename) { - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } else { - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, - "storage.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } - - if (!xml) { - if (virGetLastError() == NULL) - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("failed to parse xml document")); - goto cleanup; - } + xmlDocPtr xml; - node = xmlDocGetRootElement(xml); - if (node == NULL) { - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("missing root element")); - goto cleanup; + if ((xml = virXMLParse(filename, xmlStr, "storage.xml"))) { + ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - ret = virStorageVolDefParseNode(pool, xml, node); - - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - return ret; - - cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - return NULL; } virStorageVolDefPtr -- 1.7.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 501a00c..be0f15e 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -73,9 +73,10 @@ cpuCompareXML(virCPUDefPtr host, virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; - doc = xmlParseMemory(xml, strlen(xml)); + if (!(doc = virXMLParseString(xml, "cpu.xml"))) + goto cleanup; - if (doc == NULL || (ctxt = xmlXPathNewContext(doc)) == NULL) { + if ((ctxt = xmlXPathNewContext(doc)) == NULL) { virReportOOMError(); goto cleanup; } @@ -268,8 +269,10 @@ cpuBaselineXML(const char **xmlCPUs, goto no_memory; for (i = 0; i < ncpus; i++) { - doc = xmlParseMemory(xmlCPUs[i], strlen(xmlCPUs[i])); - if (doc == NULL || (ctxt = xmlXPathNewContext(doc)) == NULL) + if (!(doc = virXMLParseString(xmlCPUs[i], "cpu.xml"))) + goto error; + + if ((ctxt = xmlXPathNewContext(doc)) == NULL) goto no_memory; ctxt->node = xmlDocGetRootElement(doc); -- 1.7.0
On Wed, Feb 24, 2010 at 11:07:31PM +0100, Jiri Denemark wrote:
Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places.
looking very quickly, this looks sensible, but I can't fully review this right now, ACK on principle though, maybe this can wait for 0.7.8. 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/
Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places.
looking very quickly, this looks sensible, but I can't fully review this right now, ACK on principle though, maybe this can wait for 0.7.8.
In fact, the only visible change made by this patch set is better error reporting when XML passed to virConnectBaselineCPU and virConnectCompareCPU are not well-formed. So I guess it can wait for 0.7.8. Jirka
On Thu, Feb 25, 2010 at 01:12:47PM +0100, Daniel Veillard wrote:
On Wed, Feb 24, 2010 at 11:07:31PM +0100, Jiri Denemark wrote:
Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places.
looking very quickly, this looks sensible, but I can't fully review this right now, ACK on principle though, maybe this can wait for 0.7.8.
Okay, reread them, looks just fine by me, ACK, let's push them once 0.7.7 is out :-) 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/
Almost identical XML parsing code for either strings or files was copied 15 times. Since it was needed on two other places for parsing CPU XML strings with correct error reporting, I decided to factor the parsing code out to xml.c and used this one implementation in all other places.
looking very quickly, this looks sensible, but I can't fully review this right now, ACK on principle though, maybe this can wait for 0.7.8.
Okay, reread them, looks just fine by me, ACK, let's push them once 0.7.7 is out :-)
As 0.7.7 has been out for some time now, I pushed these patches. Actually, they were very close to being forgotten forever :-) Jirka
participants (2)
-
Daniel Veillard -
Jiri Denemark