From c957c0bac7f9785ae2a7520a9f693eeda0ff545b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 24 Dec 2006 00:29:20 +0000 Subject: Code review for XML patch. Instill a bit of sanity in the location of the XmlExpr code in various lists, use a representation that has some hope of reverse-listing correctly (though it's still a de-escaping function shy of correctness), generally try to make it look more like Postgres coding conventions. --- src/backend/utils/adt/xml.c | 137 ++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 89 deletions(-) (limited to 'src/backend/utils/adt/xml.c') diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index d765c8657fb..a6dc1a6e5f0 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.2 2006/12/23 04:56:50 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.3 2006/12/24 00:29:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -41,15 +41,6 @@ #ifdef USE_LIBXML -/* - * A couple of useful macros (similar to ones from libxml/parse.c) - */ -#define CMP4( s, c1, c2, c3, c4 ) \ - ( ((unsigned char *) s)[ 0 ] == c1 && ((unsigned char *) s)[ 1 ] == c2 && \ - ((unsigned char *) s)[ 2 ] == c3 && ((unsigned char *) s)[ 3 ] == c4 ) -#define CMP5( s, c1, c2, c3, c4, c5 ) \ - ( CMP4( s, c1, c2, c3, c4 ) && ((unsigned char *) s)[ 4 ] == c5 ) - #define PG_XML_DEFAULT_URI "dummy.xml" #define XML_ERRBUF_SIZE 200 @@ -177,31 +168,18 @@ xmlcomment(PG_FUNCTION_ARGS) Datum -xmlparse(PG_FUNCTION_ARGS) +texttoxml(PG_FUNCTION_ARGS) { -#ifdef USE_LIBXML - text *data; - bool is_document; - bool preserve_whitespace; - - data = PG_GETARG_TEXT_P(0); + text *data = PG_GETARG_TEXT_P(0); - if (PG_NARGS() >= 2) - is_document = PG_GETARG_BOOL(1); - else - is_document = false; + PG_RETURN_XML_P(xmlparse(data, false, true)); +} - if (PG_NARGS() >= 3) - preserve_whitespace = PG_GETARG_BOOL(2); - else - /* - * Since the XMLPARSE grammar makes STRIP WHITESPACE the - * default, this argument should really default to false. But - * until we have actually implemented whitespace stripping, - * this would be annoying. - */ - preserve_whitespace = true; +xmltype * +xmlparse(text *data, bool is_document, bool preserve_whitespace) +{ +#ifdef USE_LIBXML if (!preserve_whitespace) ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -213,111 +191,93 @@ xmlparse(PG_FUNCTION_ARGS) * valies defined by internal DTD are applied'. As for external * DTDs, we try to support them too, (see SQL/XML:10.16.7.e) */ - xml_parse(data, XML_PARSE_DTDATTR, is_document); /* assume that ERROR occurred if parsing failed */ + xml_parse(data, XML_PARSE_DTDATTR, is_document); - PG_RETURN_XML_P(data); + return (xmltype *) data; #else NO_XML_SUPPORT(); - return 0; + return NULL; #endif } -Datum -xmlpi(PG_FUNCTION_ARGS) +xmltype * +xmlpi(char *target, text *arg) { #ifdef USE_LIBXML - char *target = NameStr(*PG_GETARG_NAME(0)); + xmltype *result; StringInfoData buf; - if (strlen(target) >= 3 - && (target[0] == 'x' || target[0] == 'X') - && (target[1] == 'm' || target[1] == 'M') - && (target[2] == 'l' || target[2] == 'L')) - { + if (pg_strncasecmp(target, "xml", 3) == 0) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), + (errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION), errmsg("invalid XML processing instruction"), errdetail("XML processing instruction target name cannot start with \"xml\"."))); - } initStringInfo(&buf); - appendStringInfo(&buf, " 1) + appendStringInfo(&buf, "")) + string = DatumGetCString(DirectFunctionCall1(textout, + PointerGetDatum(arg))); + if (strstr(string, "?>") != NULL) ereport(ERROR, (errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION), errmsg("invalid XML processing instruction"), errdetail("XML processing instruction cannot contain \"?>\"."))); - appendStringInfoString(&buf, " "); + appendStringInfoChar(&buf, ' '); appendStringInfoString(&buf, string); + pfree(string); } appendStringInfoString(&buf, "?>"); - PG_RETURN_XML_P(stringinfo_to_xmltype(&buf)); + result = stringinfo_to_xmltype(&buf); + pfree(buf.data); + return result; #else NO_XML_SUPPORT(); - return 0; + return NULL; #endif } -Datum -xmlroot(PG_FUNCTION_ARGS) +xmltype * +xmlroot(xmltype *data, text *version, int standalone) { #ifdef USE_LIBXML - xmltype *data; - text *version; - int standalone; + xmltype *result; StringInfoData buf; - if (PG_ARGISNULL(0)) - PG_RETURN_NULL(); - else - data = PG_GETARG_XML_P(0); - - if (PG_ARGISNULL(1)) - version = NULL; - else - version = PG_GETARG_TEXT_P(1); - - if (PG_ARGISNULL(2)) - standalone = 0; - else - { - bool tmp = PG_GETARG_BOOL(2); - standalone = (tmp ? 1 : -1); - } + initStringInfo(&buf); /* * FIXME: This is probably supposed to be cleverer if there * already is an XML preamble. */ - initStringInfo(&buf); - appendStringInfo(&buf,""); appendStringInfoText(&buf, (text *) data); - PG_RETURN_XML_P(stringinfo_to_xmltype(&buf)); + result = stringinfo_to_xmltype(&buf); + pfree(buf.data); + return result; #else NO_XML_SUPPORT(); - return 0; + return NULL; #endif } @@ -456,7 +416,7 @@ xml_parse(text *data, int opts, bool is_document) /* first, we try to parse the string as XML doc, then, as XML chunk */ ereport(DEBUG3, (errmsg("string to parse: %s", string))); - if (len > 4 && CMP5(string, '<', '?', 'x', 'm', 'l')) + if (len >= 5 && strncmp((char *) string, "