Re: review: xml_is_well_formed

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