From: Jacob Champion <pchampion@vmware.com>
To: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: michael@paquier.xyz <michael@paquier.xyz>
Subject: [PATCH] Make jsonapi usable from libpq
Date: Tue, 22 Jun 2021 22:59:37 +0000
Message-ID: <a250d475ba1c0cc0efb7dfec8e538fcc77cdcb8e.camel@vmware.com> (raw)
(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.camel@vmware.comAttachments:download all (2 files, .tar.gz)0001-src-common-remove-logging-from-jsonapi-for-shlib.patch (text/x-patch, 1.7K) download
From 0541598e4f0bad1b9ff41a4640ec69491b393d54 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@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/Makefileindex 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.cindex 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
0002-common-jsonapi-always-palloc-the-error-strings.patch (text/x-patch, 2.2K) download
From 5ad4b3c7835fe9e0f284702ec7b827c27770854e Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pchampion@vmware.com, michael@paquier.xyz
Subject: Re: [PATCH] Make jsonapi usable from libpq
In-Reply-To: <a250d475ba1c0cc0efb7dfec8e538fcc77cdcb8e.camel@vmware.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox