[PATCH 0/5] util: xml: cleanup virxml.h and virXMLParseHelper
Peter Krempa (5): util: virxml: Fix formatting of virxml.h virXMLParseHelper: Sync argument names between declaration and definition util: xml: Register autoptr cleanup function for 'xmlParserCtxt' virXMLParseHelper: Rework error reporting virXMLParseHelper: Refactor cleanup src/util/virxml.c | 45 ++++++----- src/util/virxml.h | 193 +++++++++++++++++++++++++++------------------- 2 files changed, 137 insertions(+), 101 deletions(-) -- 2.30.2
Remove the "block" formatting of function declarations and use uniform spacing. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.h | 192 +++++++++++++++++++++++++++------------------- 1 file changed, 112 insertions(+), 80 deletions(-) diff --git a/src/util/virxml.h b/src/util/virxml.h index de171dce12..84fa4f4baf 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -32,60 +32,80 @@ xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) G_GNUC_WARN_UNUSED_RESULT; -int virXPathBoolean(const char *xpath, - xmlXPathContextPtr ctxt); -char * virXPathString(const char *xpath, - xmlXPathContextPtr ctxt); -char * virXPathStringLimit(const char *xpath, - size_t maxlen, - xmlXPathContextPtr ctxt); -int virXPathNumber(const char *xpath, - xmlXPathContextPtr ctxt, - double *value); -int virXPathInt(const char *xpath, - xmlXPathContextPtr ctxt, - int *value); -int virXPathUInt(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned int *value); -int virXPathLong(const char *xpath, - xmlXPathContextPtr ctxt, - long *value); -int virXPathULong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value); -int virXPathULongLong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long long *value); -int virXPathLongLong(const char *xpath, - xmlXPathContextPtr ctxt, - long long *value); -int virXPathLongHex(const char *xpath, - xmlXPathContextPtr ctxt, - long *value); -int virXPathULongHex(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value); -xmlNodePtr virXPathNode(const char *xpath, - xmlXPathContextPtr ctxt); -int virXPathNodeSet(const char *xpath, - xmlXPathContextPtr ctxt, - xmlNodePtr **list); -char * virXMLPropString(xmlNodePtr node, - const char *name); -char * virXMLPropStringLimit(xmlNodePtr node, - const char *name, - size_t maxlen); -char * virXMLNodeContentString(xmlNodePtr node); +int +virXPathBoolean(const char *xpath, + xmlXPathContextPtr ctxt); +char * +virXPathString(const char *xpath, + xmlXPathContextPtr ctxt); +char * +virXPathStringLimit(const char *xpath, + size_t maxlen, + xmlXPathContextPtr ctxt); +int +virXPathNumber(const char *xpath, + xmlXPathContextPtr ctxt, + double *value); +int +virXPathInt(const char *xpath, + xmlXPathContextPtr ctxt, + int *value); +int +virXPathUInt(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int *value); +int +virXPathLong(const char *xpath, + xmlXPathContextPtr ctxt, + long *value); +int +virXPathULong(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned long *value); +int +virXPathULongLong(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned long long *value); +int +virXPathLongLong(const char *xpath, + xmlXPathContextPtr ctxt, + long long *value); +int +virXPathLongHex(const char *xpath, + xmlXPathContextPtr ctxt, + long *value); +int +virXPathULongHex(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned long *value); +xmlNodePtr +virXPathNode(const char *xpath, + xmlXPathContextPtr ctxt); +int +virXPathNodeSet(const char *xpath, + xmlXPathContextPtr ctxt, + xmlNodePtr **list); +char * +virXMLPropString(xmlNodePtr node, + const char *name); +char * +virXMLPropStringLimit(xmlNodePtr node, + const char *name, + size_t maxlen); +char * +virXMLNodeContentString(xmlNodePtr node); /* Internal function; prefer the macros below. */ -xmlDocPtr virXMLParseHelper(int domcode, - const char *filename, - const char *xmlStr, - const char *url, - xmlXPathContextPtr *pctxt); - -const char *virXMLPickShellSafeComment(const char *str1, const char *str2); +xmlDocPtr +virXMLParseHelper(int domcode, + const char *filename, + const char *xmlStr, + const char *url, + xmlXPathContextPtr *pctxt); + +const char * +virXMLPickShellSafeComment(const char *str1, + const char *str2); /** * virXMLParse: * @filename: file to parse, or NULL for string parsing @@ -164,32 +184,41 @@ const char *virXMLPickShellSafeComment(const char *str1, const char *str2); #define virXMLParseFileCtxt(filename, pctxt) \ virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt) -int virXMLSaveFile(const char *path, - const char *warnName, - const char *warnCommand, - const char *xml); - -char *virXMLNodeToString(xmlDocPtr doc, xmlNodePtr node); +int +virXMLSaveFile(const char *path, + const char *warnName, + const char *warnCommand, + const char *xml); -bool virXMLNodeNameEqual(xmlNodePtr node, - const char *name); +char * +virXMLNodeToString(xmlDocPtr doc, + xmlNodePtr node); -xmlNodePtr virXMLFindChildNodeByNs(xmlNodePtr root, - const char *uri); +bool +virXMLNodeNameEqual(xmlNodePtr node, + const char *name); -int virXMLExtractNamespaceXML(xmlNodePtr root, - const char *uri, - char **doc); +xmlNodePtr +virXMLFindChildNodeByNs(xmlNodePtr root, + const char *uri); -int virXMLInjectNamespace(xmlNodePtr node, +int +virXMLExtractNamespaceXML(xmlNodePtr root, const char *uri, - const char *key); + char **doc); -void virXMLNodeSanitizeNamespaces(xmlNodePtr node); +int +virXMLInjectNamespace(xmlNodePtr node, + const char *uri, + const char *key); -int virXMLCheckIllegalChars(const char *nodeName, - const char *str, - const char *illegal); +void +virXMLNodeSanitizeNamespaces(xmlNodePtr node); + +int +virXMLCheckIllegalChars(const char *nodeName, + const char *str, + const char *illegal); struct _virXMLValidator { xmlRelaxNGParserCtxtPtr rngParser; @@ -255,9 +284,11 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlXPathContext, xmlXPathFreeContext); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlBuffer, xmlBufferFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlNode, xmlFreeNode); -typedef int (*virXMLNamespaceParse)(xmlXPathContextPtr ctxt, void **nsdata); +typedef int (*virXMLNamespaceParse)(xmlXPathContextPtr ctxt, + void **nsdata); typedef void (*virXMLNamespaceFree)(void *nsdata); -typedef int (*virXMLNamespaceFormat)(virBuffer *buf, void *nsdata); +typedef int (*virXMLNamespaceFormat)(virBuffer *buf, + void *nsdata); typedef const char *(*virXMLNamespaceHref)(void); struct _virXMLNamespace { @@ -276,13 +307,14 @@ int virXMLNamespaceRegister(xmlXPathContextPtr ctxt, virXMLNamespace const *ns); -int virParseScaledValue(const char *xpath, - const char *units_xpath, - xmlXPathContextPtr ctxt, - unsigned long long *val, - unsigned long long scale, - unsigned long long max, - bool required); +int +virParseScaledValue(const char *xpath, + const char *units_xpath, + xmlXPathContextPtr ctxt, + unsigned long long *val, + unsigned long long scale, + unsigned long long max, + bool required); xmlBufferPtr virXMLBufferCreate(void); -- 2.30.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virxml.h b/src/util/virxml.h index 84fa4f4baf..fce1c28e89 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -101,7 +101,7 @@ virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, const char *url, - xmlXPathContextPtr *pctxt); + xmlXPathContextPtr *ctxt); const char * virXMLPickShellSafeComment(const char *str1, -- 2.30.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virxml.h b/src/util/virxml.h index fce1c28e89..b1feeeb164 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -283,6 +283,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlDoc, xmlFreeDoc); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlXPathContext, xmlXPathFreeContext); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlBuffer, xmlBufferFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlNode, xmlFreeNode); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlParserCtxt, xmlFreeParserCtxt); typedef int (*virXMLNamespaceParse)(xmlXPathContextPtr ctxt, void **nsdata); -- 2.30.2
Move the reporting of parsing error on the error path of the parser as other code paths report their own errors already. Additionally prefer printing the 'url' as document name if provided instead of "[inline data]" as that usually gives a better hint at least which kind of XML is being parsed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 117f50f2bf..029b3d646e 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -788,6 +788,14 @@ virXMLParseHelper(int domcode, struct virParserData private; xmlParserCtxtPtr pctxt; xmlDocPtr xml = NULL; + const char *docname; + + if (filename) + docname = filename; + else if (url) + docname = url; + else + docname = "[inline data]"; /* Set up a parser context so we can catch the details of XML errors. */ pctxt = xmlNewParserCtxt(); @@ -807,8 +815,16 @@ virXMLParseHelper(int domcode, XML_PARSE_NONET | XML_PARSE_NOWARNING); } - if (!xml) + + if (!xml) { + if (virGetLastErrorCode() == VIR_ERR_OK) { + virGenericReportError(domcode, VIR_ERR_XML_ERROR, + _("failed to parse xml document '%s'"), + docname); + } + goto error; + } if (xmlDocGetRootElement(xml) == NULL) { virGenericReportError(domcode, VIR_ERR_INTERNAL_ERROR, @@ -832,11 +848,6 @@ virXMLParseHelper(int domcode, xmlFreeDoc(xml); xml = NULL; - if (virGetLastErrorCode() == VIR_ERR_OK) { - virGenericReportError(domcode, VIR_ERR_XML_ERROR, - _("failed to parse xml document '%s'"), - filename ? filename : "[inline data]"); - } goto cleanup; } -- 2.30.2
Switch @xml and @pctxt to g_autofree and get rid of the "error" and "cleanup" labels. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 029b3d646e..58d640546f 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -786,8 +786,8 @@ virXMLParseHelper(int domcode, xmlXPathContextPtr *ctxt) { struct virParserData private; - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; + g_autoptr(xmlParserCtxt) pctxt = NULL; + g_autoptr(xmlDoc) xml = NULL; const char *docname; if (filename) @@ -823,32 +823,24 @@ virXMLParseHelper(int domcode, docname); } - goto error; + return NULL; } if (xmlDocGetRootElement(xml) == NULL) { virGenericReportError(domcode, VIR_ERR_INTERNAL_ERROR, "%s", _("missing root element")); - goto error; + + return NULL; } if (ctxt) { if (!(*ctxt = virXMLXPathContextNew(xml))) - goto error; + return NULL; (*ctxt)->node = xmlDocGetRootElement(xml); } - cleanup: - xmlFreeParserCtxt(pctxt); - - return xml; - - error: - xmlFreeDoc(xml); - xml = NULL; - - goto cleanup; + return g_steal_pointer(&xml); } const char *virXMLPickShellSafeComment(const char *str1, const char *str2) -- 2.30.2
On Fri, 2021-04-16 at 10:07 +0200, Peter Krempa wrote:
Peter Krempa (5): util: virxml: Fix formatting of virxml.h virXMLParseHelper: Sync argument names between declaration and definition util: xml: Register autoptr cleanup function for 'xmlParserCtxt' virXMLParseHelper: Rework error reporting virXMLParseHelper: Refactor cleanup
src/util/virxml.c | 45 ++++++----- src/util/virxml.h | 193 +++++++++++++++++++++++++++----------------- -- 2 files changed, 137 insertions(+), 101 deletions(-)
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
participants (2)
-
Peter Krempa -
Tim Wiederhake