[libvirt] [PATCH RFC 1/2] XML: Improve XML parsing error messages

[Daniel Veillard]

On Mon, Sep 05, 2011 at 02:33:39PM +0200, Peter Krempa wrote:
> This patch modifies error handling function for the XML parser provided
> by libxml2.
> 
> Originaly only a line number and error message were logged. With this
> new error handler function, the user is provided with a more complex
> description of the parsing error.
> 
> Context of the error is printed in libXML2 style and filename of the
> file, that caused the error is printed. Example of an parse error:
> 
> 13:41:36.262: 16032: error : catchXMLError:706 :
> /etc/libvirt/qemu/rh_bad.xml:58: Opening and ending tag mismatch: name
> line 2 and domain
> </domain>
> ---------^
> 
> Context of the error gives the user hints that may help to quickly
> locate a corrupt xml file.
> 
> fixes BZs:
> ----------
> Bug 708735 - [RFE] Show column and line on XML parsing error
> https://bugzilla.redhat.com/show_bug.cgi?id=708735
> 
> Bug 726771 - libvirt does not specify problem file if persistent xml is
> invalid
> https://bugzilla.redhat.com/show_bug.cgi?id=726771
> ---
>  src/util/xml.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 76 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/xml.c b/src/util/xml.c
> index d301af6..b0942da 100644
> --- a/src/util/xml.c
> +++ b/src/util/xml.c
> @@ -633,28 +633,92 @@ virXPathNodeSet(const char *xpath,
>   * catchXMLError:
>   *
>   * Called from SAX on parsing errors in the XML.
> + *
> + * This version is heavily based on xmlParserPrintFileContextInternal from libxml2.
>   */
>  static void
>  catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
>  {
> -    int domcode = VIR_FROM_XML;
>      xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx;
> 
> -    if (ctxt) {
> -        if (ctxt->_private)
> +    const xmlChar *cur, *base;
> +    unsigned int n, col;	/* GCC warns if signed, because compared with sizeof() */
> +    int domcode = VIR_FROM_XML;
> +
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    char *contextstr = NULL;
> +    char *pointerstr = NULL;
> +
> +
> +    /* conditions for error printing */
> +    if (!ctxt ||
> +        (virGetLastError() != NULL) ||
> +        ctxt->input == NULL ||
> +        ctxt->lastError.level != XML_ERR_FATAL ||
> +        ctxt->lastError.message == NULL)
> +        return;
> +
> +    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);
> -        }
> +
> +    cur = ctxt->input->cur;
> +    base = ctxt->input->base;
> +
> +    /* skip backwards over any end-of-lines */
> +    while ((cur > base) && ((*(cur) == '\n') || (*(cur) == '\r'))) {
> +        cur--;
>      }
> -}
> 
> +    /* search backwards for beginning-of-line (to max buff size) */
> +    while ((cur > base) && (*(cur) != '\n') && (*(cur) != '\r'))
> +        cur--;
> +    if ((*(cur) == '\n') || (*(cur) == '\r')) cur++;
> +
> +    /* calculate the error position in terms of the current position */
> +    col = ctxt->input->cur - cur;
> +
> +    /* search forward for end-of-line (to max buff size) */
> +    /* copy selected text to our buffer */
> +    while ((*cur != 0) && (*(cur) != '\n') && (*(cur) != '\r')) {
> +        virBufferAddChar(&buf, *cur++);
> +    }
> +
> +    /* create blank line with problem pointer */
> +    contextstr = virBufferContentAndReset(&buf);
> +
> +    /* (leave buffer space for pointer + line terminator) */
> +    for  (n = 0; (n<col) && (contextstr[n] != 0); n++) {
> +        if (contextstr[n] == '\t')
> +            virBufferAddChar(&buf, '\t');
> +        else
> +            virBufferAddChar(&buf, '-');
> +    }
> +
> +    virBufferAddChar(&buf, '^');
> +
> +    pointerstr = virBufferContentAndReset(&buf);
> +
> +    if (ctxt->lastError.file) {
> +        virGenericReportError(domcode, VIR_ERR_XML_DETAIL,
> +                              _("%s:%d: %s%s\n%s"),
> +                              ctxt->lastError.file,
> +                              ctxt->lastError.line,
> +                              ctxt->lastError.message,
> +                              contextstr,
> +                              pointerstr);
> +    } else {
> +         virGenericReportError(domcode, VIR_ERR_XML_DETAIL,
> +                              _("at line %d: %s%s\n%s"),
> +                              ctxt->lastError.line,
> +                              ctxt->lastError.message,
> +                              contextstr,
> +                              pointerstr);
> +    }
> +
> +    VIR_FREE(contextstr);
> +    VIR_FREE(pointerstr);
> +}
> 

  ACK, looks okay, it's a bit convoluted, but the libxml2 routine
got an awful lot of use over the years and works, so ...

  pushed, thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/