(This is split off from my work on OAUTHBEARER [1].)
The jsonapi in src/common can't currently be compiled into libpq. The
first patch here removes the dependency on pg_log_fatal(), which is not
available to the sharedlib. The second patch makes sure that all of the
return values from json_errdetail() can be pfree'd, to avoid long-
running leaks.
In the original thread, Michael Paquier commented:
> +# define check_stack_depth()
> +# ifdef JSONAPI_NO_LOG
> +# define json_log_and_abort(...) \
> + do { fprintf(stderr, __VA_ARGS__); exit(1); } while(0)
> +# else
> In patch 0002, this is the wrong approach. libpq will not be able to
> feed on such reports, and you cannot use any of the APIs from the
> palloc() family either as these just fail on OOM. libpq should be
> able to know about the error, and would fill in the error back to the
> application. This abstraction is not necessary on HEAD as
> pg_verifybackup is fine with this level of reporting. My rough guess
> is that we will need to split the existing jsonapi.c into two files,
> one that can be used in shared libraries and a second that handles the
> errors.
Hmm. I'm honestly hesitant to start splitting files apart, mostly
because json_log_and_abort() is only called from two places, and both
those places are triggered by programmer error as opposed to user
error.
Would it make more sense to switch to an fprintf-and-abort case, to
match the approach taken by PGTHREAD_ERROR and the out-of-memory
conditions in fe-print.c? Or is there already precedent for handling
can't-happen code paths with in-band errors, through the PGconn?
--Jacob
[1]
https://www.postgresql.org/message-id/d1b467a78e0e36ed85a09adf979d04cf124a9d4b.ca...@vmware.com
From 0541598e4f0bad1b9ff41a4640ec69491b393d54 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Mon, 3 May 2021 11:15:15 -0700
Subject: [PATCH 2/7] src/common: remove logging from jsonapi for shlib
The can't-happen code in jsonapi was pulling in logging code, which for
libpq is not included.
---
src/common/Makefile | 4 ++++
src/common/jsonapi.c | 11 ++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/common/Makefile b/src/common/Makefile
index 38a8599337..6f1039bc78 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -28,6 +28,10 @@ subdir = src/common
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
+# For use in shared libraries, jsonapi needs to not link in any logging
+# functions.
+override CFLAGS_SL += -DJSONAPI_NO_LOG
+
# don't include subdirectory-path-dependent -I and -L switches
STD_CPPFLAGS := $(filter-out -I$(top_srcdir)/src/include -I$(top_builddir)/src/include,$(CPPFLAGS))
STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/common -L$(top_builddir)/src/port,$(LDFLAGS))
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 1bf38d7b42..6b6001b118 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -27,11 +27,16 @@
#endif
#ifdef FRONTEND
-#define check_stack_depth()
-#define json_log_and_abort(...) \
+# define check_stack_depth()
+# ifdef JSONAPI_NO_LOG
+# define json_log_and_abort(...) \
+ do { fprintf(stderr, __VA_ARGS__); exit(1); } while(0)
+# else
+# define json_log_and_abort(...) \
do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
+# endif
#else
-#define json_log_and_abort(...) elog(ERROR, __VA_ARGS__)
+# define json_log_and_abort(...) elog(ERROR, __VA_ARGS__)
#endif
/*
--
2.25.1
From 5ad4b3c7835fe9e0f284702ec7b827c27770854e Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Mon, 3 May 2021 15:38:26 -0700
Subject: [PATCH 3/7] common/jsonapi: always palloc the error strings
...so that client code can pfree() to avoid memory leaks in long-running
operations.
---
src/common/jsonapi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 6b6001b118..f7304f584f 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -1089,7 +1089,7 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
return psprintf(_("Expected JSON value, but found \"%s\"."),
extract_token(lex));
case JSON_EXPECTED_MORE:
- return _("The input string ended unexpectedly.");
+ return pstrdup(_("The input string ended unexpectedly."));
case JSON_EXPECTED_OBJECT_FIRST:
return psprintf(_("Expected string or \"}\", but found \"%s\"."),
extract_token(lex));
@@ -1103,16 +1103,16 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
return psprintf(_("Token \"%s\" is invalid."),
extract_token(lex));
case JSON_UNICODE_CODE_POINT_ZERO:
- return _("\\u0000 cannot be converted to text.");
+ return pstrdup(_("\\u0000 cannot be converted to text."));
case JSON_UNICODE_ESCAPE_FORMAT:
- return _("\"\\u\" must be followed by four hexadecimal digits.");
+ return pstrdup(_("\"\\u\" must be followed by four hexadecimal digits."));
case JSON_UNICODE_HIGH_ESCAPE:
/* note: this case is only reachable in frontend not backend */
- return _("Unicode escape values cannot be used for code point values above 007F when the encoding is not UTF8.");
+ return pstrdup(_("Unicode escape values cannot be used for code point values above 007F when the encoding is not UTF8."));
case JSON_UNICODE_HIGH_SURROGATE:
- return _("Unicode high surrogate must not follow a high surrogate.");
+ return pstrdup(_("Unicode high surrogate must not follow a high surrogate."));
case JSON_UNICODE_LOW_SURROGATE:
- return _("Unicode low surrogate must follow a high surrogate.");
+ return pstrdup(_("Unicode low surrogate must follow a high surrogate."));
}
/*
--
2.25.1
