Skip site navigation (1) Skip section navigation (2)

Re: review: xml_is_well_formed

From: Mike Fowler <mike(at)mlfowler(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: review: xml_is_well_formed
Date: 2010-07-30 11:50:25
Message-ID: 4C52BC81.9050209@mlfowler.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Hi Pavel,

Thanks for taking the time to review my patch. Attached is a new version 
addressing your concerns.

On 29/07/10 14:21, Pavel Stehule wrote:
> I have a few issues:
> * broken regress test (fedora 13 - xmllint: using libxml version 20707)
>
> postgres=# SELECT xml_is_well_formed('<pg:foo
> xmlns:pg="http://postgresql.org/stuff";>bar</pg:foo>');
>   xml_is_well_formed
> --------------------
>   f
> (1 row)
>
> this xml is broken - but in regress tests is ok
>
> [pavel(at)pavel-stehule ~]$ xmllint xxx
> xxx:1: parser error : error parsing attribute name
> <pg:foo xmlns:pg="http://postgresql.org/stuff";>bar</pg:foo>
>    

This is a problem that was observered recently by Thom Brown - the 
commit fest webapp adds the semicolon after the quote. If you look at 
the attachment I sent in a email client you'll find there is no 
semicolon. The patch attached here will also not have the semicolon.

> * xml_is_well_formed returns true for simple text
>
> postgres=# SELECT xml_is_well_formed('ssss');
>   xml_is_well_formed
> --------------------
>   t
> (1 row)
>
> it is probably wrong result - is it ok??
>    

Yes this is OK, pure text is valid XML content.

> * I don't understand to this fragment
>
>         PG_TRY();
> +       {
> +               size_t          count;
> +               xmlChar    *version = NULL;
> +               int                     standalone = -1;
> +.
> +               res_code = parse_xml_decl(string,&count,&version,
> NULL,&standalone);
> +               if (res_code != 0)
> +                       xml_ereport_by_code(ERROR, ERRCODE_INVALID_XML_CONTENT,
> +                                                 "invalid XML
> content: invalid XML declaration",
> +                                                       res_code);
> +.
> +               doc = xmlNewDoc(version);
> +               doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
> +               doc->standalone = 1;
>
> why? This function can raise exception when declaration is wrong. It
> is wrong - I think, this function should returns false instead
> exception.
>
>    

You're quite right,  I should be returning false here and not causing an 
exception. I have corrected this in the attached patch.

Regards,

-- 
Mike Fowler
Registered Linux user: 379787


Attachment: xml_is_well_formed-4.patch
Description: text/x-diff (10.3 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Pavel StehuleDate: 2010-07-30 12:51:53
Subject: Re: review: xml_is_well_formed
Previous:From: Vincenzo RomanoDate: 2010-07-30 11:40:31
Subject: Re: On Scalability

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group