Thread: PG versus libxml2 2.12.x
Buildfarm member caiman has been failing build for a couple weeks now.
The reason turns out to be that recent libxml2 has decided to throw
a "const" into the signature required for custom error handlers.
(API compatibility? What's that?)
I don't mind adopting the "const" --- it's a good idea in isolation.
The trouble is in fixing our code to work with both old and new
libxml2 versions. We could thrash around with a configure test or
something, but I think the most expedient answer is just to insert
some explicit casts, as shown in the attached. It's possible though
that some compilers will throw a cast-away-const warning. I'm
not seeing any, but ...
Also, I'm seeing a deprecation warning in contrib/xml2/xpath.c
for
xmlLoadExtDtdDefaultValue = 1;
I'm not sure why that's still there, given that we disabled external
DTD access ages ago. I propose we just remove it.
In short, I suggest the attached.
regards, tom lane
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index a967257546..b999b1f706 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -74,8 +74,6 @@ pgxml_parser_init(PgXmlStrictness strictness)
/* Initialize libxml */
xmlInitParser();
- xmlLoadExtDtdDefaultValue = 1;
-
return xmlerrcxt;
}
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index f869c680af..a6734f3550 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -124,7 +124,7 @@ static xmlParserInputPtr xmlPgEntityLoader(const char *URL, const char *ID,
xmlParserCtxtPtr ctxt);
static void xml_errsave(Node *escontext, PgXmlErrorContext *errcxt,
int sqlcode, const char *msg);
-static void xml_errorHandler(void *data, xmlErrorPtr error);
+static void xml_errorHandler(void *data, const xmlError *error);
static int errdetail_for_xml_code(int code);
static void chopStringInfoNewlines(StringInfo str);
static void appendStringInfoLineSeparator(StringInfo str);
@@ -1196,7 +1196,8 @@ pg_xml_init(PgXmlStrictness strictness)
errcxt->saved_errcxt = xmlGenericErrorContext;
#endif
- xmlSetStructuredErrorFunc((void *) errcxt, xml_errorHandler);
+ xmlSetStructuredErrorFunc((void *) errcxt,
+ (xmlStructuredErrorFunc) xml_errorHandler);
/*
* Verify that xmlSetStructuredErrorFunc set the context variable we
@@ -2024,7 +2025,7 @@ xml_errsave(Node *escontext, PgXmlErrorContext *errcxt,
* Error handler for libxml errors and warnings
*/
static void
-xml_errorHandler(void *data, xmlErrorPtr error)
+xml_errorHandler(void *data, const xmlError *error)
{
PgXmlErrorContext *xmlerrcxt = (PgXmlErrorContext *) data;
xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) error->ctxt;
@@ -4803,7 +4804,8 @@ XmlTableFetchRow(TableFuncScanState *state)
xtCxt = GetXmlTableBuilderPrivateData(state, "XmlTableFetchRow");
/* Propagate our own error context to libxml2 */
- xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt, xml_errorHandler);
+ xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt,
+ (xmlStructuredErrorFunc) xml_errorHandler);
if (xtCxt->xpathobj == NULL)
{
@@ -4857,7 +4859,8 @@ XmlTableGetValue(TableFuncScanState *state, int colnum,
xtCxt->xpathobj->nodesetval != NULL);
/* Propagate our own error context to libxml2 */
- xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt, xml_errorHandler);
+ xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt,
+ (xmlStructuredErrorFunc) xml_errorHandler);
*isnull = false;
@@ -5000,7 +5003,8 @@ XmlTableDestroyOpaque(TableFuncScanState *state)
xtCxt = GetXmlTableBuilderPrivateData(state, "XmlTableDestroyOpaque");
/* Propagate our own error context to libxml2 */
- xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt, xml_errorHandler);
+ xmlSetStructuredErrorFunc((void *) xtCxt->xmlerrcxt,
+ (xmlStructuredErrorFunc) xml_errorHandler);
if (xtCxt->xpathscomp != NULL)
{
On 2024-01-27 Sa 14:04, Tom Lane wrote: > Buildfarm member caiman has been failing build for a couple weeks now. > The reason turns out to be that recent libxml2 has decided to throw > a "const" into the signature required for custom error handlers. > (API compatibility? What's that?) > > I don't mind adopting the "const" --- it's a good idea in isolation. > The trouble is in fixing our code to work with both old and new > libxml2 versions. We could thrash around with a configure test or > something, but I think the most expedient answer is just to insert > some explicit casts, as shown in the attached. It's possible though > that some compilers will throw a cast-away-const warning. I'm > not seeing any, but ... > > Also, I'm seeing a deprecation warning in contrib/xml2/xpath.c > for > > xmlLoadExtDtdDefaultValue = 1; > > I'm not sure why that's still there, given that we disabled external > DTD access ages ago. I propose we just remove it. > > In short, I suggest the attached. > > Looks reasonable. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 27.01.24 20:04, Tom Lane wrote: > Buildfarm member caiman has been failing build for a couple weeks now. > The reason turns out to be that recent libxml2 has decided to throw > a "const" into the signature required for custom error handlers. > (API compatibility? What's that?) > > I don't mind adopting the "const" --- it's a good idea in isolation. > The trouble is in fixing our code to work with both old and new > libxml2 versions. We could thrash around with a configure test or > something, but I think the most expedient answer is just to insert > some explicit casts, as shown in the attached. It's possible though > that some compilers will throw a cast-away-const warning. I'm > not seeing any, but ... In PL/Tcl, we used to have these CONST84 and CONST86 things, for similar reasons. Maybe that would be another approach.
Peter Eisentraut <peter@eisentraut.org> writes:
> On 27.01.24 20:04, Tom Lane wrote:
>> I don't mind adopting the "const" --- it's a good idea in isolation.
>> The trouble is in fixing our code to work with both old and new
>> libxml2 versions. We could thrash around with a configure test or
>> something, but I think the most expedient answer is just to insert
>> some explicit casts, as shown in the attached. It's possible though
>> that some compilers will throw a cast-away-const warning. I'm
>> not seeing any, but ...
> In PL/Tcl, we used to have these CONST84 and CONST86 things, for similar
> reasons. Maybe that would be another approach.
Yeah, if the simple cast approach turns out to create warnings,
we'll have to fall back on using actually different declarations.
I'm hoping to not have to go there.
regards, tom lane
I wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> In PL/Tcl, we used to have these CONST84 and CONST86 things, for similar
>> reasons. Maybe that would be another approach.
> Yeah, if the simple cast approach turns out to create warnings,
> we'll have to fall back on using actually different declarations.
> I'm hoping to not have to go there.
Actually ... what I really want to avoid is adding a configure test.
The alternative to that would be an #if test on LIBXML_VERSION,
which I'd initially not wanted to do ... but I now notice that
we already have one of those for a nearby purpose (coping with a
different change in libxml2's error APIs). So adding another one
of those doesn't seem so bad after all. I now like the attached
approach better.
regards, tom lane
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index a967257546..b999b1f706 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -74,8 +74,6 @@ pgxml_parser_init(PgXmlStrictness strictness)
/* Initialize libxml */
xmlInitParser();
- xmlLoadExtDtdDefaultValue = 1;
-
return xmlerrcxt;
}
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index f869c680af..3e24aba546 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -67,6 +67,16 @@
#if LIBXML_VERSION >= 20704
#define HAVE_XMLSTRUCTUREDERRORCONTEXT 1
#endif
+
+/*
+ * libxml2 2.12 decided to insert "const" into the error handler API.
+ */
+#if LIBXML_VERSION >= 21200
+#define PgXmlErrorPtr const xmlError *
+#else
+#define PgXmlErrorPtr xmlErrorPtr
+#endif
+
#endif /* USE_LIBXML */
#include "access/htup_details.h"
@@ -124,7 +134,7 @@ static xmlParserInputPtr xmlPgEntityLoader(const char *URL, const char *ID,
xmlParserCtxtPtr ctxt);
static void xml_errsave(Node *escontext, PgXmlErrorContext *errcxt,
int sqlcode, const char *msg);
-static void xml_errorHandler(void *data, xmlErrorPtr error);
+static void xml_errorHandler(void *data, PgXmlErrorPtr error);
static int errdetail_for_xml_code(int code);
static void chopStringInfoNewlines(StringInfo str);
static void appendStringInfoLineSeparator(StringInfo str);
@@ -2024,7 +2034,7 @@ xml_errsave(Node *escontext, PgXmlErrorContext *errcxt,
* Error handler for libxml errors and warnings
*/
static void
-xml_errorHandler(void *data, xmlErrorPtr error)
+xml_errorHandler(void *data, PgXmlErrorPtr error)
{
PgXmlErrorContext *xmlerrcxt = (PgXmlErrorContext *) data;
xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) error->ctxt;