From a12333eed2af3399cd70170c070a6aa9a4550bc5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 28 Feb 2010 19:51:37 +0000 Subject: [PATCH] Assorted code cleanup for contrib/xml2. No change in functionality, just make it a bit less ugly in places. --- contrib/xml2/xpath.c | 304 +++++++++++++++++---------------------- contrib/xml2/xslt_proc.c | 22 +-- 2 files changed, 145 insertions(+), 181 deletions(-) diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c index b1a4e9ac9d..b2458e9b11 100644 --- a/contrib/xml2/xpath.c +++ b/contrib/xml2/xpath.c @@ -1,5 +1,5 @@ /* - * $PostgreSQL: pgsql/contrib/xml2/xpath.c,v 1.25 2010/01/17 12:11:25 mha Exp $ + * $PostgreSQL: pgsql/contrib/xml2/xpath.c,v 1.26 2010/02/28 19:51:37 tgl Exp $ * * Parser interface for DOM-based parser (libxml) rather than * stream-based SAX-type parser @@ -24,28 +24,7 @@ PG_MODULE_MAGIC; -/* declarations */ - -static void *pgxml_palloc(size_t size); -static void *pgxml_repalloc(void *ptr, size_t size); -static void pgxml_pfree(void *ptr); -static char *pgxml_pstrdup(const char *string); -static void pgxml_errorHandler(void *ctxt, const char *msg,...); - -void elog_error(int level, char *explain, int force); -void pgxml_parser_init(void); - -static xmlChar *pgxmlNodeSetToText(xmlNodeSetPtr nodeset, - xmlChar *toptagname, xmlChar *septagname, - xmlChar *plainsep); - -text *pgxml_result_to_text(xmlXPathObjectPtr res, xmlChar *toptag, - xmlChar *septag, xmlChar *plainsep); - -xmlChar *pgxml_texttoxmlchar(text *textstring); - -static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath); - +/* externally accessible functions */ Datum xml_is_well_formed(PG_FUNCTION_ARGS); Datum xml_encode_special_chars(PG_FUNCTION_ARGS); @@ -56,11 +35,34 @@ Datum xpath_bool(PG_FUNCTION_ARGS); Datum xpath_list(PG_FUNCTION_ARGS); Datum xpath_table(PG_FUNCTION_ARGS); -/* Global variables */ -char *errbuf; /* per line error buffer */ -char *pgxml_errorMsg = NULL; /* overall error message */ +/* these are exported for use by xslt_proc.c */ + +void elog_error(const char *explain, bool force); +void pgxml_parser_init(void); + +/* local declarations */ + +static void *pgxml_palloc(size_t size); +static void *pgxml_repalloc(void *ptr, size_t size); +static void pgxml_pfree(void *ptr); +static char *pgxml_pstrdup(const char *string); +static void pgxml_errorHandler(void *ctxt, const char *msg,...); + +static xmlChar *pgxmlNodeSetToText(xmlNodeSetPtr nodeset, + xmlChar *toptagname, xmlChar *septagname, + xmlChar *plainsep); + +static text *pgxml_result_to_text(xmlXPathObjectPtr res, xmlChar *toptag, + xmlChar *septag, xmlChar *plainsep); + +static xmlChar *pgxml_texttoxmlchar(text *textstring); + +static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath); + + +/* Global variables */ +static char *pgxml_errorMsg = NULL; /* overall error message */ -#define ERRBUF_SIZE 200 /* memory handling passthrough functions (e.g. palloc, pstrdup are currently macros, and the others might become so...) */ @@ -92,75 +94,77 @@ pgxml_pstrdup(const char *string) return pstrdup(string); } -/* The error handling function. This formats an error message and sets +/* + * The error handling function. This formats an error message and sets * a flag - an ereport will be issued prior to return */ - static void pgxml_errorHandler(void *ctxt, const char *msg,...) { + char errbuf[1024]; /* per line error buffer */ va_list args; + /* Format the message */ va_start(args, msg); - vsnprintf(errbuf, ERRBUF_SIZE, msg, args); + vsnprintf(errbuf, sizeof(errbuf), msg, args); va_end(args); - /* Now copy the argument across */ + /* Store in, or append to, pgxml_errorMsg */ if (pgxml_errorMsg == NULL) pgxml_errorMsg = pstrdup(errbuf); else { - int32 xsize = strlen(pgxml_errorMsg); - - pgxml_errorMsg = repalloc(pgxml_errorMsg, - (size_t) (xsize + strlen(errbuf) + 1)); - strncpy(&pgxml_errorMsg[xsize - 1], errbuf, strlen(errbuf)); - pgxml_errorMsg[xsize + strlen(errbuf) - 1] = '\0'; + size_t oldsize = strlen(pgxml_errorMsg); + size_t newsize = strlen(errbuf); + /* + * We intentionally discard the last char of the existing message, + * which should be a carriage return. (XXX wouldn't it be saner + * to keep it?) + */ + pgxml_errorMsg = repalloc(pgxml_errorMsg, oldsize + newsize); + memcpy(&pgxml_errorMsg[oldsize - 1], errbuf, newsize); + pgxml_errorMsg[oldsize + newsize - 1] = '\0'; } - memset(errbuf, 0, ERRBUF_SIZE); } -/* This function reports the current message at the level specified */ +/* + * This function ereports the current message if any. If force is true + * then an error is thrown even if pgxml_errorMsg hasn't been set. + */ void -elog_error(int level, char *explain, int force) +elog_error(const char *explain, bool force) { - if (force || (pgxml_errorMsg != NULL)) + if (force || pgxml_errorMsg != NULL) { if (pgxml_errorMsg == NULL) - { - ereport(level, (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION), - errmsg("%s", explain))); - } + ereport(ERROR, + (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION), + errmsg("%s", explain))); else - { - ereport(level, (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION), - errmsg("%s:%s", explain, pgxml_errorMsg))); - pfree(pgxml_errorMsg); - } + ereport(ERROR, + (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION), + errmsg("%s: %s", explain, pgxml_errorMsg))); } } +/* + * Initialize for xml parsing. + */ void -pgxml_parser_init() +pgxml_parser_init(void) { - /* - * This code could also set parser settings from user-supplied info. - * Quite how these settings are made is another matter :) - */ - - xmlMemSetup(pgxml_pfree, pgxml_palloc, pgxml_repalloc, pgxml_pstrdup); - xmlInitParser(); - + /* Set up error handling */ + pgxml_errorMsg = NULL; xmlSetGenericErrorFunc(NULL, pgxml_errorHandler); + /* Replace libxml memory handling (DANGEROUS) */ + xmlMemSetup(pgxml_pfree, pgxml_palloc, pgxml_repalloc, pgxml_pstrdup); + + /* Initialize libxml */ + xmlInitParser(); + xmlSubstituteEntitiesDefault(1); xmlLoadExtDtdDefaultValue = 1; - - pgxml_errorMsg = NULL; - - errbuf = palloc(200); - memset(errbuf, 0, 200); - } @@ -171,10 +175,9 @@ PG_FUNCTION_INFO_V1(xml_is_well_formed); Datum xml_is_well_formed(PG_FUNCTION_ARGS) { - /* called as xml_is_well_formed(document) */ - xmlDocPtr doctree; text *t = PG_GETARG_TEXT_P(0); /* document buffer */ int32 docsize = VARSIZE(t) - VARHDRSZ; + xmlDocPtr doctree; pgxml_parser_init(); @@ -184,8 +187,8 @@ xml_is_well_formed(PG_FUNCTION_ARGS) xmlCleanupParser(); PG_RETURN_BOOL(false); /* i.e. not well-formed */ } - xmlCleanupParser(); xmlFreeDoc(doctree); + xmlCleanupParser(); PG_RETURN_BOOL(true); } @@ -215,28 +218,23 @@ xml_encode_special_chars(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(tout); } -static xmlChar - * +/* + * Function translates a nodeset into a text representation + * + * iterates over each node in the set and calls xmlNodeDump to write it to + * an xmlBuffer -from which an xmlChar * string is returned. + * + * each representation is surrounded by ... + * + * plainsep is an ordinary (not tag) separator - if used, then nodes are + * cast to string as output method + */ +static xmlChar * pgxmlNodeSetToText(xmlNodeSetPtr nodeset, xmlChar *toptagname, xmlChar *septagname, xmlChar *plainsep) { - /* Function translates a nodeset into a text representation */ - - /* - * iterates over each node in the set and calls xmlNodeDump to write it to - * an xmlBuffer -from which an xmlChar * string is returned. - */ - - /* each representation is surrounded by ... */ - - /* - * plainsep is an ordinary (not tag) seperator - if used, then nodes are - * cast to string as output method - */ - - xmlBufferPtr buf; xmlChar *result; int i; @@ -253,7 +251,6 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset, { for (i = 0; i < nodeset->nodeNr; i++) { - if (plainsep != NULL) { xmlBufferWriteCHAR(buf, @@ -265,8 +262,6 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset, } else { - - if ((septagname != NULL) && (xmlStrlen(septagname) > 0)) { xmlBufferWriteChar(buf, "<"); @@ -303,19 +298,18 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset, /* Translate a PostgreSQL "varlena" -i.e. a variable length parameter * into the libxml2 representation */ - -xmlChar * +static xmlChar * pgxml_texttoxmlchar(text *textstring) { return (xmlChar *) text_to_cstring(textstring); } -/* Public visible XPath functions */ +/* Publicly visible XPath functions */ -/* This is a "raw" xpath function. Check that it returns child elements +/* + * This is a "raw" xpath function. Check that it returns child elements * properly */ - PG_FUNCTION_INFO_V1(xpath_nodeset); Datum @@ -325,8 +319,7 @@ xpath_nodeset(PG_FUNCTION_ARGS) *toptag, *septag; int32 pathsize; - text - *xpathsupp, + text *xpathsupp, *xpres; /* PG_GETARG_TEXT_P(0) is document buffer */ @@ -339,8 +332,7 @@ xpath_nodeset(PG_FUNCTION_ARGS) xpath = pgxml_texttoxmlchar(xpathsupp); - xpres = pgxml_result_to_text( - pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), + xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), toptag, septag, NULL); /* xmlCleanupParser(); done by result_to_text routine */ @@ -351,9 +343,10 @@ xpath_nodeset(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(xpres); } -/* The following function is almost identical, but returns the elements in */ -/* a list. */ - +/* + * The following function is almost identical, but returns the elements in + * a list. + */ PG_FUNCTION_INFO_V1(xpath_list); Datum @@ -362,8 +355,7 @@ xpath_list(PG_FUNCTION_ARGS) xmlChar *xpath, *plainsep; int32 pathsize; - text - *xpathsupp, + text *xpathsupp, *xpres; /* PG_GETARG_TEXT_P(0) is document buffer */ @@ -375,8 +367,7 @@ xpath_list(PG_FUNCTION_ARGS) xpath = pgxml_texttoxmlchar(xpathsupp); - xpres = pgxml_result_to_text( - pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), + xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), NULL, NULL, plainsep); /* xmlCleanupParser(); done by result_to_text routine */ @@ -395,8 +386,7 @@ xpath_string(PG_FUNCTION_ARGS) { xmlChar *xpath; int32 pathsize; - text - *xpathsupp, + text *xpathsupp, *xpres; /* PG_GETARG_TEXT_P(0) is document buffer */ @@ -416,8 +406,7 @@ xpath_string(PG_FUNCTION_ARGS) xpath[pathsize + 7] = ')'; xpath[pathsize + 8] = '\0'; - xpres = pgxml_result_to_text( - pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), + xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath), NULL, NULL, NULL); xmlCleanupParser(); @@ -436,9 +425,7 @@ xpath_number(PG_FUNCTION_ARGS) { xmlChar *xpath; int32 pathsize; - text - *xpathsupp; - + text *xpathsupp; float4 fRes; xmlXPathObjectPtr res; @@ -465,7 +452,6 @@ xpath_number(PG_FUNCTION_ARGS) PG_RETURN_NULL(); PG_RETURN_FLOAT4(fRes); - } @@ -476,9 +462,7 @@ xpath_bool(PG_FUNCTION_ARGS) { xmlChar *xpath; int32 pathsize; - text - *xpathsupp; - + text *xpathsupp; int bRes; xmlXPathObjectPtr res; @@ -502,26 +486,21 @@ xpath_bool(PG_FUNCTION_ARGS) bRes = xmlXPathCastToBoolean(res); xmlCleanupParser(); PG_RETURN_BOOL(bRes); - } /* Core function to evaluate XPath query */ -xmlXPathObjectPtr +static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath) { - xmlDocPtr doctree; xmlXPathContextPtr ctxt; xmlXPathObjectPtr res; - xmlXPathCompExprPtr comppath; - int32 docsize; - docsize = VARSIZE(document) - VARHDRSZ; pgxml_parser_init(); @@ -535,16 +514,13 @@ pgxml_xpath(text *document, xmlChar *xpath) ctxt = xmlXPathNewContext(doctree); ctxt->node = xmlDocGetRootElement(doctree); - /* compile the path */ comppath = xmlXPathCompile(xpath); if (comppath == NULL) { xmlCleanupParser(); xmlFreeDoc(doctree); - elog_error(ERROR, "XPath Syntax Error", 1); - - return NULL; + elog_error("XPath Syntax Error", true); } /* Now evaluate the path expression. */ @@ -563,8 +539,7 @@ pgxml_xpath(text *document, xmlChar *xpath) return res; } -text - * +static text * pgxml_result_to_text(xmlXPathObjectPtr res, xmlChar *toptag, xmlChar *septag, @@ -595,7 +570,6 @@ pgxml_result_to_text(xmlXPathObjectPtr res, xpresstr = xmlStrdup((const xmlChar *) ""); } - /* Now convert this result back to text */ xpres = cstring_to_text((char *) xpresstr); @@ -605,27 +579,33 @@ pgxml_result_to_text(xmlXPathObjectPtr res, xmlFree(xpresstr); - elog_error(ERROR, "XPath error", 0); - + elog_error("XPath error", false); return xpres; } -/* xpath_table is a table function. It needs some tidying (as do the +/* + * xpath_table is a table function. It needs some tidying (as do the * other functions here! */ - PG_FUNCTION_INFO_V1(xpath_table); Datum xpath_table(PG_FUNCTION_ARGS) { -/* SPI (input tuple) support */ + /* Function parameters */ + char *pkeyfield = text_to_cstring(PG_GETARG_TEXT_PP(0)); + char *xmlfield = text_to_cstring(PG_GETARG_TEXT_PP(1)); + char *relname = text_to_cstring(PG_GETARG_TEXT_PP(2)); + char *xpathset = text_to_cstring(PG_GETARG_TEXT_PP(3)); + char *condition = text_to_cstring(PG_GETARG_TEXT_PP(4)); + + /* SPI (input tuple) support */ SPITupleTable *tuptable; HeapTuple spi_tuple; TupleDesc spi_tupdesc; -/* Output tuple (tuplestore) support */ + /* Output tuple (tuplestore) support */ Tuplestorestate *tupstore = NULL; TupleDesc ret_tupdesc; HeapTuple ret_tuple; @@ -635,13 +615,6 @@ xpath_table(PG_FUNCTION_ARGS) MemoryContext per_query_ctx; MemoryContext oldcontext; -/* Function parameters */ - char *pkeyfield = text_to_cstring(PG_GETARG_TEXT_PP(0)); - char *xmlfield = text_to_cstring(PG_GETARG_TEXT_PP(1)); - char *relname = text_to_cstring(PG_GETARG_TEXT_PP(2)); - char *xpathset = text_to_cstring(PG_GETARG_TEXT_PP(3)); - char *condition = text_to_cstring(PG_GETARG_TEXT_PP(4)); - char **values; xmlChar **xpaths; char *pos; @@ -655,7 +628,6 @@ xpath_table(PG_FUNCTION_ARGS) int rownr; /* For issuing multiple rows from one original * document */ int had_values; /* To determine end of nodeset results */ - StringInfoData query_buf; /* We only have a valid tuple description in table function mode */ @@ -743,30 +715,28 @@ xpath_table(PG_FUNCTION_ARGS) pkeyfield, xmlfield, relname, - condition - ); - + condition); if ((ret = SPI_connect()) < 0) elog(ERROR, "xpath_table: SPI_connect returned %d", ret); if ((ret = SPI_exec(query_buf.data, 0)) != SPI_OK_SELECT) - elog(ERROR, "xpath_table: SPI execution failed for query %s", query_buf.data); + elog(ERROR, "xpath_table: SPI execution failed for query %s", + query_buf.data); proc = SPI_processed; /* elog(DEBUG1,"xpath_table: SPI returned %d rows",proc); */ tuptable = SPI_tuptable; spi_tupdesc = tuptable->tupdesc; -/* Switch out of SPI context */ + /* Switch out of SPI context */ MemoryContextSwitchTo(oldcontext); - -/* Check that SPI returned correct result. If you put a comma into one of - * the function parameters, this will catch it when the SPI query returns - * e.g. 3 columns. - */ - + /* + * Check that SPI returned correct result. If you put a comma into one of + * the function parameters, this will catch it when the SPI query returns + * e.g. 3 columns. + */ if (spi_tupdesc->natts != 2) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -774,10 +744,12 @@ xpath_table(PG_FUNCTION_ARGS) errdetail("Expected two columns in SPI result, got %d.", spi_tupdesc->natts))); } -/* Setup the parser. Beware that this must happen in the same context as the - * cleanup - which means that any error from here on must do cleanup to - * ensure that the entity table doesn't get freed by being out of context. - */ + /* + * Setup the parser. Beware that this must happen in the same context as + * the cleanup - which means that any error from here on must do cleanup + * to ensure that the entity table doesn't get freed by being out of + * context. + */ pgxml_parser_init(); /* For each row i.e. document returned from SPI */ @@ -785,13 +757,10 @@ xpath_table(PG_FUNCTION_ARGS) { char *pkey; char *xmldoc; - xmlDocPtr doctree; xmlXPathContextPtr ctxt; xmlXPathObjectPtr res; xmlChar *resstr; - - xmlXPathCompExprPtr comppath; /* Extract the row data as C Strings */ @@ -801,10 +770,9 @@ xpath_table(PG_FUNCTION_ARGS) /* * Clear the values array, so that not-well-formed documents return - * NULL in all columns. + * NULL in all columns. Note that this also means that spare columns + * will be NULL. */ - - /* Note that this also means that spare columns will be NULL. */ for (j = 0; j < ret_tupdesc->natts; j++) values[j] = NULL; @@ -835,7 +803,6 @@ xpath_table(PG_FUNCTION_ARGS) had_values = 0; for (j = 0; j < numpaths; j++) { - ctxt = xmlXPathNewContext(doctree); ctxt->node = xmlDocGetRootElement(doctree); xmlSetGenericErrorFunc(ctxt, pgxml_errorHandler); @@ -847,7 +814,7 @@ xpath_table(PG_FUNCTION_ARGS) xmlCleanupParser(); xmlFreeDoc(doctree); - elog_error(ERROR, "XPath Syntax Error", 1); + elog_error("XPath Syntax Error", true); PG_RETURN_NULL(); /* Keep compiler happy */ } @@ -882,7 +849,6 @@ xpath_table(PG_FUNCTION_ARGS) resstr = xmlStrdup((const xmlChar *) ""); } - /* * Insert this into the appropriate column in the * result tuple. @@ -891,6 +857,7 @@ xpath_table(PG_FUNCTION_ARGS) } xmlXPathFreeContext(ctxt); } + /* Now add the tuple to the output, if there is one. */ if (had_values) { @@ -900,9 +867,7 @@ xpath_table(PG_FUNCTION_ARGS) } rownr++; - } while (had_values); - } xmlFreeDoc(doctree); @@ -914,7 +879,7 @@ xpath_table(PG_FUNCTION_ARGS) } xmlCleanupParser(); -/* Needed to flag completeness in 7.3.1. 7.4 defines it as a no-op. */ + tuplestore_donestoring(tupstore); SPI_finish(); @@ -929,5 +894,4 @@ xpath_table(PG_FUNCTION_ARGS) * expecting. */ return (Datum) 0; - } diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c index 0477bca779..ddf90f9c71 100644 --- a/contrib/xml2/xslt_proc.c +++ b/contrib/xml2/xslt_proc.c @@ -1,5 +1,5 @@ /* - * $PostgreSQL: pgsql/contrib/xml2/xslt_proc.c,v 1.16 2009/07/10 00:32:00 tgl Exp $ + * $PostgreSQL: pgsql/contrib/xml2/xslt_proc.c,v 1.17 2010/02/28 19:51:37 tgl Exp $ * * XSLT processing functions (requiring libxslt) * @@ -27,16 +27,16 @@ #include -/* declarations to come from xpath.c */ -extern void elog_error(int level, char *explain, int force); -extern void pgxml_parser_init(); -extern xmlChar *pgxml_texttoxmlchar(text *textstring); - -/* local defs */ -static void parse_params(const char **params, text *paramstr); +/* externally accessible functions */ Datum xslt_process(PG_FUNCTION_ARGS); +/* declarations to come from xpath.c */ +extern void elog_error(const char *explain, bool force); +extern void pgxml_parser_init(void); + +/* local defs */ +static void parse_params(const char **params, text *paramstr); #define MAXPARAMS 20 /* must be even, see parse_params() */ @@ -80,7 +80,7 @@ xslt_process(PG_FUNCTION_ARGS) if (doctree == NULL) { xmlCleanupParser(); - elog_error(ERROR, "error parsing XML document", 0); + elog_error("error parsing XML document", false); PG_RETURN_NULL(); } @@ -94,7 +94,7 @@ xslt_process(PG_FUNCTION_ARGS) { xmlFreeDoc(doctree); xmlCleanupParser(); - elog_error(ERROR, "error parsing stylesheet as XML document", 0); + elog_error("error parsing stylesheet as XML document", false); PG_RETURN_NULL(); } @@ -109,7 +109,7 @@ xslt_process(PG_FUNCTION_ARGS) xmlFreeDoc(doctree); xsltCleanupGlobals(); xmlCleanupParser(); - elog_error(ERROR, "failed to parse stylesheet", 0); + elog_error("failed to parse stylesheet", false); PG_RETURN_NULL(); }