On 2023-Jul-26, Amit Langote wrote:
> Some refactoring to export json(b) conversion functions
>
> This is to export datum_to_json(), datum_to_jsonb(), and
> jsonb_from_cstring(), though the last one is exported as
> jsonb_from_text().
After this commit, Coverity started complaining that
datum_to_jsonb_internal() leaks the JsonLexContext here
754 │ case JSONTYPE_CAST:
755 │ case JSONTYPE_JSON:
756 │ {
757 │ /* parse the json right into the existing result
object */
758 │ JsonLexContext *lex;
759 │ JsonSemAction sem;
760 │ text *json = DatumGetTextPP(val);
761 │
762 │ lex = makeJsonLexContext(json, true);
763 │
764 │ memset(, 0, sizeof(sem));
765 │
766 │ sem.semstate = (void *) result;
767 │
768 │ sem.object_start = jsonb_in_object_start;
769 │ sem.array_start = jsonb_in_array_start;
770 │ sem.object_end = jsonb_in_object_end;
771 │ sem.array_end = jsonb_in_array_end;
772 │ sem.scalar = jsonb_in_scalar;
773 │ sem.object_field_start =
jsonb_in_object_field_start;
774 │
775 │ pg_parse_json_or_ereport(lex, );
776 │ }
777 │ break;
Admittedly, our support code for this is not great, since we have no
clean way to free those resources. Some places like json_object_keys
are freeing everything manually (even though in that particular case
it's unnecessary, since that one runs in a memcxt that's going to be
cleaned up shortly afterwards).
One idea that Tom floated was to allow the JsonLexContext to be
optionally stack-allocated. That reduces palloc() traffic; but some
callers do need it to be palloc'ed. Here's a patch that does it that
way, and adds a freeing routine that knows what to do in either case.
It may make sense to do some further analysis and remove useless free
calls.
It may make sense to change the structs that contain JsonLexContext *
so that they directly embed JsonLexContext instead. That would further
reduce palloc'ing.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end." (2nd Commandment for C programmers)
>From df8b71428481f718ac6bb60a3ca5969a6876da7e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Thu, 3 Aug 2023 11:44:15 +0200
Subject: [PATCH] JsonLexContext allocation/free
---
src/backend/utils/adt/json.c | 38
src/backend/utils/adt/jsonb.c| 13 +--
src/backend/utils/adt/jsonfuncs.c| 106 +--
src/bin/pg_verifybackup/parse_manifest.c | 4 +-
src/common/jsonapi.c | 43 +++--
src/include/common/jsonapi.h | 23 +++--
src/include/utils/jsonfuncs.h| 2 +-
7 files changed, 146 insertions(+), 83 deletions(-)
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index e405791f5d..27f9a51228 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -106,11 +106,11 @@ json_in(PG_FUNCTION_ARGS)
{
char *json = PG_GETARG_CSTRING(0);
text *result = cstring_to_text(json);
- JsonLexContext *lex;
+ JsonLexContext lex;
/* validate it */
- lex = makeJsonLexContext(result, false);
- if (!pg_parse_json_or_errsave(lex, , fcinfo->context))
+ makeJsonLexContext(, result, false);
+ if (!pg_parse_json_or_errsave(, , fcinfo->context))
PG_RETURN_NULL();
/* Internal representation is the same as text */
@@ -152,13 +152,13 @@ json_recv(PG_FUNCTION_ARGS)
StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
char *str;
int nbytes;
- JsonLexContext *lex;
+ JsonLexContext lex;
str = pq_getmsgtext(buf, buf->len - buf->cursor, );
/* Validate it. */
- lex = makeJsonLexContextCstringLen(str, nbytes, GetDatabaseEncoding(), false);
- pg_parse_json_or_ereport(lex, );
+ makeJsonLexContextCstringLen(, str, nbytes, GetDatabaseEncoding(), false);
+ pg_parse_json_or_ereport(, );
PG_RETURN_TEXT_P(cstring_to_text_with_len(str, nbytes));
}
@@ -1625,14 +1625,16 @@ json_unique_object_field_start(void *_state, char *field, bool isnull)
bool
json_validate(text *json, bool check_unique_keys, bool throw_error)
{
- JsonLexContext *lex = makeJsonLexContext(json, check_unique_keys);
+ JsonLexContext lex;
JsonSemAction uniqueSemAction = {0};
JsonUniqueParsingState state;
JsonParseErrorType result;
+ makeJsonLexContext(, json, check_unique_keys);
+
if (check_unique_keys)
{
- state.lex = lex;
+ state.lex =
state.stack = NULL;
state.id_counter = 0;
state.unique = true;
@@ -1644,12 +1646,12 @@ json_validate(text *json, bool check_unique_keys, bool throw_error)
