[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] xml: add another convenience function



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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]