jsonapi type fixups

Started by Peter Eisentrautover 1 year ago4 messages
#1Peter Eisentraut
peter@eisentraut.org
3 attachment(s)

I have this patch series that fixes up the types of the new incremental
JSON API a bit. Specifically, it uses "const" throughout so that the
top-level entry points such as pg_parse_json_incremental() can declare
their arguments as const char * instead of just char *. This just
works, it doesn't require any new casting tricks. In fact, it removes a
few unconstify() calls.

Also, a few arguments and variables that relate to object sizes should
be size_t rather than int. At this point, this mainly makes the API
better self-documenting. I don't think it actually works to parse
larger than 2 GB chunks (not tested).

Attachments:

0001-jsonapi-Use-size_t.patchtext/plain; charset=UTF-8; name=0001-jsonapi-Use-size_t.patchDownload
From 24546d9043f0bbde185dafca74f9eb7225267a68 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 18 Jun 2024 13:42:23 +0200
Subject: [PATCH 1/3] jsonapi: Use size_t

Use size_t instead of int for object sizes in the jsonapi.  This makes
the API better self-documenting.
---
 src/common/jsonapi.c                | 24 ++++++++++++------------
 src/common/parse_manifest.c         |  2 +-
 src/include/common/jsonapi.h        |  8 ++++----
 src/include/common/parse_manifest.h |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 172310f6f19..f71c1c54b2a 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -85,7 +85,7 @@ struct JsonParserStack
 {
 	int			stack_size;
 	char	   *prediction;
-	int			pred_index;
+	size_t		pred_index;
 	/* these two are indexed by lex_level */
 	char	  **fnames;
 	bool	   *fnull;
@@ -212,7 +212,7 @@ static char JSON_PROD_GOAL[] = {JSON_TOKEN_END, JSON_NT_JSON, 0};
 
 static inline JsonParseErrorType json_lex_string(JsonLexContext *lex);
 static inline JsonParseErrorType json_lex_number(JsonLexContext *lex, char *s,
-												 bool *num_err, int *total_len);
+												 bool *num_err, size_t *total_len);
 static inline JsonParseErrorType parse_scalar(JsonLexContext *lex, JsonSemAction *sem);
 static JsonParseErrorType parse_object_field(JsonLexContext *lex, JsonSemAction *sem);
 static JsonParseErrorType parse_object(JsonLexContext *lex, JsonSemAction *sem);
@@ -269,10 +269,10 @@ lex_expect(JsonParseContext ctx, JsonLexContext *lex, JsonTokenType token)
  * str is of length len, and need not be null-terminated.
  */
 bool
-IsValidJsonNumber(const char *str, int len)
+IsValidJsonNumber(const char *str, size_t len)
 {
 	bool		numeric_error;
-	int			total_len;
+	size_t		total_len;
 	JsonLexContext dummy_lex;
 
 	if (len <= 0)
@@ -324,7 +324,7 @@ IsValidJsonNumber(const char *str, int len)
  */
 JsonLexContext *
 makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
-							 int len, int encoding, bool need_escapes)
+							 size_t len, int encoding, bool need_escapes)
 {
 	if (lex == NULL)
 	{
@@ -650,7 +650,7 @@ JsonParseErrorType
 pg_parse_json_incremental(JsonLexContext *lex,
 						  JsonSemAction *sem,
 						  char *json,
-						  int len,
+						  size_t len,
 						  bool is_last)
 {
 	JsonTokenType tok;
@@ -888,7 +888,7 @@ pg_parse_json_incremental(JsonLexContext *lex,
 							}
 							else
 							{
-								int			tlen = (lex->token_terminator - lex->token_start);
+								ptrdiff_t	tlen = (lex->token_terminator - lex->token_start);
 
 								pstack->scalar_val = palloc(tlen + 1);
 								memcpy(pstack->scalar_val, lex->token_start, tlen);
@@ -1332,7 +1332,7 @@ json_lex(JsonLexContext *lex)
 		 * recursive call
 		 */
 		StringInfo	ptok = &(lex->inc_state->partial_token);
-		int			added = 0;
+		size_t		added = 0;
 		bool		tok_done = false;
 		JsonLexContext dummy_lex;
 		JsonParseErrorType partial_result;
@@ -1354,7 +1354,7 @@ json_lex(JsonLexContext *lex)
 					break;
 			}
 
-			for (int i = 0; i < lex->input_length; i++)
+			for (size_t i = 0; i < lex->input_length; i++)
 			{
 				char		c = lex->input[i];
 
@@ -1382,7 +1382,7 @@ json_lex(JsonLexContext *lex)
 
 				bool		numend = false;
 
-				for (int i = 0; i < lex->input_length && !numend; i++)
+				for (size_t i = 0; i < lex->input_length && !numend; i++)
 				{
 					char		cc = lex->input[i];
 
@@ -1418,7 +1418,7 @@ json_lex(JsonLexContext *lex)
 			 * {null, false, true} literals as well as any trailing
 			 * alphanumeric junk on non-string tokens.
 			 */
-			for (int i = added; i < lex->input_length; i++)
+			for (size_t i = added; i < lex->input_length; i++)
 			{
 				char		cc = lex->input[i];
 
@@ -1941,7 +1941,7 @@ json_lex_string(JsonLexContext *lex)
  */
 static inline JsonParseErrorType
 json_lex_number(JsonLexContext *lex, char *s,
-				bool *num_err, int *total_len)
+				bool *num_err, size_t *total_len)
 {
 	bool		error = false;
 	int			len = s - lex->input;
diff --git a/src/common/parse_manifest.c b/src/common/parse_manifest.c
index 821fba3967a..373a4f6c00f 100644
--- a/src/common/parse_manifest.c
+++ b/src/common/parse_manifest.c
@@ -183,7 +183,7 @@ json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incs
 
 void
 json_parse_manifest_incremental_chunk(
-									  JsonManifestParseIncrementalState *incstate, char *chunk, int size,
+									  JsonManifestParseIncrementalState *incstate, char *chunk, size_t size,
 									  bool is_last)
 {
 	JsonParseErrorType res,
diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index f1ab17fc9f2..5d3ae4e09b8 100644
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -89,7 +89,7 @@ typedef struct JsonIncrementalState JsonIncrementalState;
 typedef struct JsonLexContext
 {
 	char	   *input;
-	int			input_length;
+	size_t		input_length;
 	int			input_encoding;
 	char	   *token_start;
 	char	   *token_terminator;
@@ -158,7 +158,7 @@ extern JsonParseErrorType pg_parse_json(JsonLexContext *lex,
 extern JsonParseErrorType pg_parse_json_incremental(JsonLexContext *lex,
 													JsonSemAction *sem,
 													char *json,
-													int len,
+													size_t len,
 													bool is_last);
 
 /* the null action object used for pure validation */
@@ -193,7 +193,7 @@ extern JsonParseErrorType json_count_array_elements(JsonLexContext *lex,
  */
 extern JsonLexContext *makeJsonLexContextCstringLen(JsonLexContext *lex,
 													char *json,
-													int len,
+													size_t len,
 													int encoding,
 													bool need_escapes);
 
@@ -219,6 +219,6 @@ extern char *json_errdetail(JsonParseErrorType error, JsonLexContext *lex);
  *
  * str argument does not need to be nul-terminated.
  */
-extern bool IsValidJsonNumber(const char *str, int len);
+extern bool IsValidJsonNumber(const char *str, size_t len);
 
 #endif							/* JSONAPI_H */
diff --git a/src/include/common/parse_manifest.h b/src/include/common/parse_manifest.h
index 0d04239c38d..2777b1e9d22 100644
--- a/src/include/common/parse_manifest.h
+++ b/src/include/common/parse_manifest.h
@@ -51,7 +51,7 @@ extern void json_parse_manifest(JsonManifestParseContext *context,
 								char *buffer, size_t size);
 extern JsonManifestParseIncrementalState *json_parse_manifest_incremental_init(JsonManifestParseContext *context);
 extern void json_parse_manifest_incremental_chunk(
-												  JsonManifestParseIncrementalState *incstate, char *chunk, int size,
+												  JsonManifestParseIncrementalState *incstate, char *chunk, size_t size,
 												  bool is_last);
 extern void json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incstate);
 

base-commit: ae482a7ec521e09bb0dbfc261da6e6170a5ac29f
-- 
2.45.2

0002-jsonapi-Use-const-char.patchtext/plain; charset=UTF-8; name=0002-jsonapi-Use-const-char.patchDownload
From 919929c2aa6f78d50866484db6552cb3db381056 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 18 Jun 2024 13:42:23 +0200
Subject: [PATCH 2/3] jsonapi: Use const char *

Apply const qualifiers to char * arguments and fields throughout the
jsonapi.  This allows the top-level APIs such as
pg_parse_json_incremental() to declare their input argument as const.
It also gets rid of a few unconstify() calls.
---
 src/backend/utils/adt/jsonfuncs.c | 37 ++++++++++++++++---------------
 src/common/jsonapi.c              | 26 +++++++++++-----------
 src/include/common/jsonapi.h      | 14 ++++++------
 3 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 83125b06a43..2d7fe687d67 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -86,7 +86,7 @@ typedef struct GetState
 {
 	JsonLexContext *lex;
 	text	   *tresult;
-	char	   *result_start;
+	const char *result_start;
 	bool		normalize_results;
 	bool		next_scalar;
 	int			npath;			/* length of each path-related array */
@@ -111,7 +111,7 @@ typedef struct EachState
 	Tuplestorestate *tuple_store;
 	TupleDesc	ret_tdesc;
 	MemoryContext tmp_cxt;
-	char	   *result_start;
+	const char *result_start;
 	bool		normalize_results;
 	bool		next_scalar;
 	char	   *normalized_scalar;
@@ -125,7 +125,7 @@ typedef struct ElementsState
 	Tuplestorestate *tuple_store;
 	TupleDesc	ret_tdesc;
 	MemoryContext tmp_cxt;
-	char	   *result_start;
+	const char *result_start;
 	bool		normalize_results;
 	bool		next_scalar;
 	char	   *normalized_scalar;
@@ -138,7 +138,7 @@ typedef struct JHashState
 	const char *function_name;
 	HTAB	   *hash;
 	char	   *saved_scalar;
-	char	   *save_json_start;
+	const char *save_json_start;
 	JsonTokenType saved_token_type;
 } JHashState;
 
@@ -247,7 +247,7 @@ typedef struct PopulateRecordsetState
 	const char *function_name;
 	HTAB	   *json_hash;
 	char	   *saved_scalar;
-	char	   *save_json_start;
+	const char *save_json_start;
 	JsonTokenType saved_token_type;
 	Tuplestorestate *tuple_store;
 	HeapTupleHeader rec;
@@ -273,7 +273,7 @@ typedef struct PopulateArrayState
 {
 	JsonLexContext *lex;		/* json lexer */
 	PopulateArrayContext *ctx;	/* context */
-	char	   *element_start;	/* start of the current array element */
+	const char *element_start;	/* start of the current array element */
 	char	   *element_scalar; /* current array element token if it is a
 								 * scalar */
 	JsonTokenType element_type; /* current array element type */
@@ -295,7 +295,7 @@ typedef struct JsValue
 	{
 		struct
 		{
-			char	   *str;	/* json string */
+			const char *str;	/* json string */
 			int			len;	/* json string length or -1 if null-terminated */
 			JsonTokenType type; /* json type */
 		}			json;		/* json value */
@@ -390,7 +390,7 @@ static JsonParseErrorType elements_array_element_end(void *state, bool isnull);
 static JsonParseErrorType elements_scalar(void *state, char *token, JsonTokenType tokentype);
 
 /* turn a json object into a hash table */
-static HTAB *get_json_object_as_hash(char *json, int len, const char *funcname,
+static HTAB *get_json_object_as_hash(const char *json, int len, const char *funcname,
 									 Node *escontext);
 
 /* semantic actions for populate_array_json */
@@ -456,7 +456,7 @@ static Datum populate_record_field(ColumnIOData *col, Oid typid, int32 typmod,
 static RecordIOData *allocate_record_info(MemoryContext mcxt, int ncolumns);
 static bool JsObjectGetField(JsObject *obj, char *field, JsValue *jsv);
 static void populate_recordset_record(PopulateRecordsetState *state, JsObject *obj);
-static bool populate_array_json(PopulateArrayContext *ctx, char *json, int len);
+static bool populate_array_json(PopulateArrayContext *ctx, const char *json, int len);
 static bool populate_array_dim_jsonb(PopulateArrayContext *ctx, JsonbValue *jbv,
 									 int ndim);
 static void populate_array_report_expected_array(PopulateArrayContext *ctx, int ndim);
@@ -1181,7 +1181,7 @@ get_object_end(void *state)
 	if (lex_level == 0 && _state->npath == 0)
 	{
 		/* Special case: return the entire object */
-		char	   *start = _state->result_start;
+		const char *start = _state->result_start;
 		int			len = _state->lex->prev_token_terminator - start;
 
 		_state->tresult = cstring_to_text_with_len(start, len);
@@ -1275,7 +1275,7 @@ get_object_field_end(void *state, char *fname, bool isnull)
 			_state->tresult = (text *) NULL;
 		else
 		{
-			char	   *start = _state->result_start;
+			const char *start = _state->result_start;
 			int			len = _state->lex->prev_token_terminator - start;
 
 			_state->tresult = cstring_to_text_with_len(start, len);
@@ -1337,7 +1337,7 @@ get_array_end(void *state)
 	if (lex_level == 0 && _state->npath == 0)
 	{
 		/* Special case: return the entire array */
-		char	   *start = _state->result_start;
+		const char *start = _state->result_start;
 		int			len = _state->lex->prev_token_terminator - start;
 
 		_state->tresult = cstring_to_text_with_len(start, len);
@@ -1426,7 +1426,7 @@ get_array_element_end(void *state, bool isnull)
 			_state->tresult = (text *) NULL;
 		else
 		{
-			char	   *start = _state->result_start;
+			const char *start = _state->result_start;
 			int			len = _state->lex->prev_token_terminator - start;
 
 			_state->tresult = cstring_to_text_with_len(start, len);
@@ -1463,7 +1463,7 @@ get_scalar(void *state, char *token, JsonTokenType tokentype)
 			 * scalar token, but not whitespace before it.  Probably not worth
 			 * doing our own space-skipping to avoid that.
 			 */
-			char	   *start = _state->lex->input;
+			const char *start = _state->lex->input;
 			int			len = _state->lex->prev_token_terminator - start;
 
 			_state->tresult = cstring_to_text_with_len(start, len);
@@ -2782,7 +2782,7 @@ populate_array_scalar(void *_state, char *token, JsonTokenType tokentype)
  * Returns false if an error occurs when parsing.
  */
 static bool
-populate_array_json(PopulateArrayContext *ctx, char *json, int len)
+populate_array_json(PopulateArrayContext *ctx, const char *json, int len)
 {
 	PopulateArrayState state;
 	JsonSemAction sem;
@@ -3123,7 +3123,7 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv,
 {
 	Datum		res;
 	char	   *str = NULL;
-	char	   *json = NULL;
+	const char *json = NULL;
 
 	if (jsv->is_json)
 	{
@@ -3139,7 +3139,8 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv,
 			str[len] = '\0';
 		}
 		else
-			str = json;			/* string is already null-terminated */
+			str = unconstify(char *, json); /* string is already
+											 * null-terminated */
 
 		/* If converting to json/jsonb, make string into valid JSON literal */
 		if ((typid == JSONOID || typid == JSONBOID) &&
@@ -3784,7 +3785,7 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
  * Returns the hash table if the json is parsed successfully, NULL otherwise.
  */
 static HTAB *
-get_json_object_as_hash(char *json, int len, const char *funcname,
+get_json_object_as_hash(const char *json, int len, const char *funcname,
 						Node *escontext)
 {
 	HASHCTL		ctl;
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index f71c1c54b2a..0c6374b0fc2 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -211,7 +211,7 @@ static td_entry td_parser_table[JSON_NUM_NONTERMINALS][JSON_NUM_TERMINALS] =
 static char JSON_PROD_GOAL[] = {JSON_TOKEN_END, JSON_NT_JSON, 0};
 
 static inline JsonParseErrorType json_lex_string(JsonLexContext *lex);
-static inline JsonParseErrorType json_lex_number(JsonLexContext *lex, char *s,
+static inline JsonParseErrorType json_lex_number(JsonLexContext *lex, const char *s,
 												 bool *num_err, size_t *total_len);
 static inline JsonParseErrorType parse_scalar(JsonLexContext *lex, JsonSemAction *sem);
 static JsonParseErrorType parse_object_field(JsonLexContext *lex, JsonSemAction *sem);
@@ -290,12 +290,12 @@ IsValidJsonNumber(const char *str, size_t len)
 	 */
 	if (*str == '-')
 	{
-		dummy_lex.input = unconstify(char *, str) + 1;
+		dummy_lex.input = str + 1;
 		dummy_lex.input_length = len - 1;
 	}
 	else
 	{
-		dummy_lex.input = unconstify(char *, str);
+		dummy_lex.input = str;
 		dummy_lex.input_length = len;
 	}
 
@@ -323,7 +323,7 @@ IsValidJsonNumber(const char *str, size_t len)
  * cleanup.
  */
 JsonLexContext *
-makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
+makeJsonLexContextCstringLen(JsonLexContext *lex, const char *json,
 							 size_t len, int encoding, bool need_escapes)
 {
 	if (lex == NULL)
@@ -649,7 +649,7 @@ json_count_array_elements(JsonLexContext *lex, int *elements)
 JsonParseErrorType
 pg_parse_json_incremental(JsonLexContext *lex,
 						  JsonSemAction *sem,
-						  char *json,
+						  const char *json,
 						  size_t len,
 						  bool is_last)
 {
@@ -1308,8 +1308,8 @@ parse_array(JsonLexContext *lex, JsonSemAction *sem)
 JsonParseErrorType
 json_lex(JsonLexContext *lex)
 {
-	char	   *s;
-	char	   *const end = lex->input + lex->input_length;
+	const char *s;
+	const char *const end = lex->input + lex->input_length;
 	JsonParseErrorType result;
 
 	if (lex->incremental && lex->inc_state->partial_completed)
@@ -1593,7 +1593,7 @@ json_lex(JsonLexContext *lex)
 				break;
 			default:
 				{
-					char	   *p;
+					const char *p;
 
 					/*
 					 * We're not dealing with a string, number, legal
@@ -1671,8 +1671,8 @@ json_lex(JsonLexContext *lex)
 static inline JsonParseErrorType
 json_lex_string(JsonLexContext *lex)
 {
-	char	   *s;
-	char	   *const end = lex->input + lex->input_length;
+	const char *s;
+	const char *const end = lex->input + lex->input_length;
 	int			hi_surrogate = -1;
 
 	/* Convenience macros for error exits */
@@ -1689,7 +1689,7 @@ json_lex_string(JsonLexContext *lex)
 	} while (0)
 #define FAIL_AT_CHAR_END(code) \
 	do { \
-		char	   *term = s + pg_encoding_mblen(lex->input_encoding, s); \
+		const char	   *term = s + pg_encoding_mblen(lex->input_encoding, s); \
 		lex->token_terminator = (term <= end) ? term : end; \
 		return code; \
 	} while (0)
@@ -1854,7 +1854,7 @@ json_lex_string(JsonLexContext *lex)
 		}
 		else
 		{
-			char	   *p = s;
+			const char *p = s;
 
 			if (hi_surrogate != -1)
 				FAIL_AT_CHAR_END(JSON_UNICODE_LOW_SURROGATE);
@@ -1940,7 +1940,7 @@ json_lex_string(JsonLexContext *lex)
  * the distance from lex->input to the token end+1 is returned to *total_len.
  */
 static inline JsonParseErrorType
-json_lex_number(JsonLexContext *lex, char *s,
+json_lex_number(JsonLexContext *lex, const char *s,
 				bool *num_err, size_t *total_len)
 {
 	bool		error = false;
diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index 5d3ae4e09b8..71a491d72dc 100644
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -88,18 +88,18 @@ typedef struct JsonIncrementalState JsonIncrementalState;
 #define JSONLEX_FREE_STRVAL			(1 << 1)
 typedef struct JsonLexContext
 {
-	char	   *input;
+	const char *input;
 	size_t		input_length;
 	int			input_encoding;
-	char	   *token_start;
-	char	   *token_terminator;
-	char	   *prev_token_terminator;
+	const char *token_start;
+	const char *token_terminator;
+	const char *prev_token_terminator;
 	bool		incremental;
 	JsonTokenType token_type;
 	int			lex_level;
 	bits32		flags;
 	int			line_number;	/* line number, starting from 1 */
-	char	   *line_start;		/* where that line starts within input */
+	const char *line_start;		/* where that line starts within input */
 	JsonParserStack *pstack;
 	JsonIncrementalState *inc_state;
 	StringInfo	strval;
@@ -157,7 +157,7 @@ extern JsonParseErrorType pg_parse_json(JsonLexContext *lex,
 
 extern JsonParseErrorType pg_parse_json_incremental(JsonLexContext *lex,
 													JsonSemAction *sem,
-													char *json,
+													const char *json,
 													size_t len,
 													bool is_last);
 
@@ -192,7 +192,7 @@ extern JsonParseErrorType json_count_array_elements(JsonLexContext *lex,
  * cleanup.
  */
 extern JsonLexContext *makeJsonLexContextCstringLen(JsonLexContext *lex,
-													char *json,
+													const char *json,
 													size_t len,
 													int encoding,
 													bool need_escapes);
-- 
2.45.2

0003-parse_manifest-Use-const-char.patchtext/plain; charset=UTF-8; name=0003-parse_manifest-Use-const-char.patchDownload
From 4df85cb93e0f98e3079ec7224817fcd0f6af4d08 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 18 Jun 2024 13:42:23 +0200
Subject: [PATCH 3/3] parse_manifest: Use const char *

This adapts the manifest parsing code to take advantage of the
const-ified jsonapi.
---
 src/backend/backup/basebackup_incremental.c |  4 ++--
 src/bin/pg_combinebackup/load_manifest.c    |  6 +++---
 src/bin/pg_combinebackup/load_manifest.h    |  4 ++--
 src/bin/pg_verifybackup/pg_verifybackup.c   | 16 ++++++++--------
 src/common/parse_manifest.c                 | 12 ++++++------
 src/include/common/parse_manifest.h         |  6 +++---
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c
index b6cfeb6926a..a8691b680df 100644
--- a/src/backend/backup/basebackup_incremental.c
+++ b/src/backend/backup/basebackup_incremental.c
@@ -132,7 +132,7 @@ static void manifest_process_version(JsonManifestParseContext *context,
 static void manifest_process_system_identifier(JsonManifestParseContext *context,
 											   uint64 manifest_system_identifier);
 static void manifest_process_file(JsonManifestParseContext *context,
-								  char *pathname,
+								  const char *pathname,
 								  size_t size,
 								  pg_checksum_type checksum_type,
 								  int checksum_length,
@@ -1043,7 +1043,7 @@ manifest_process_system_identifier(JsonManifestParseContext *context,
  */
 static void
 manifest_process_file(JsonManifestParseContext *context,
-					  char *pathname, size_t size,
+					  const char *pathname, size_t size,
 					  pg_checksum_type checksum_type,
 					  int checksum_length,
 					  uint8 *checksum_payload)
diff --git a/src/bin/pg_combinebackup/load_manifest.c b/src/bin/pg_combinebackup/load_manifest.c
index d857ea00066..a7bb72d16c1 100644
--- a/src/bin/pg_combinebackup/load_manifest.c
+++ b/src/bin/pg_combinebackup/load_manifest.c
@@ -46,7 +46,7 @@
  */
 #define SH_PREFIX		manifest_files
 #define SH_ELEMENT_TYPE	manifest_file
-#define SH_KEY_TYPE		char *
+#define SH_KEY_TYPE		const char *
 #define	SH_KEY			pathname
 #define SH_HASH_KEY(tb, key)	hash_string(key)
 #define SH_EQUAL(tb, a, b)		(strcmp(a, b) == 0)
@@ -60,7 +60,7 @@ static void combinebackup_version_cb(JsonManifestParseContext *context,
 static void combinebackup_system_identifier_cb(JsonManifestParseContext *context,
 											   uint64 manifest_system_identifier);
 static void combinebackup_per_file_cb(JsonManifestParseContext *context,
-									  char *pathname, size_t size,
+									  const char *pathname, size_t size,
 									  pg_checksum_type checksum_type,
 									  int checksum_length,
 									  uint8 *checksum_payload);
@@ -267,7 +267,7 @@ combinebackup_system_identifier_cb(JsonManifestParseContext *context,
  */
 static void
 combinebackup_per_file_cb(JsonManifestParseContext *context,
-						  char *pathname, size_t size,
+						  const char *pathname, size_t size,
 						  pg_checksum_type checksum_type,
 						  int checksum_length, uint8 *checksum_payload)
 {
diff --git a/src/bin/pg_combinebackup/load_manifest.h b/src/bin/pg_combinebackup/load_manifest.h
index 8a5a70e4477..a96ae12eb8e 100644
--- a/src/bin/pg_combinebackup/load_manifest.h
+++ b/src/bin/pg_combinebackup/load_manifest.h
@@ -22,7 +22,7 @@
 typedef struct manifest_file
 {
 	uint32		status;			/* hash status */
-	char	   *pathname;
+	const char *pathname;
 	size_t		size;
 	pg_checksum_type checksum_type;
 	int			checksum_length;
@@ -31,7 +31,7 @@ typedef struct manifest_file
 
 #define SH_PREFIX		manifest_files
 #define SH_ELEMENT_TYPE	manifest_file
-#define SH_KEY_TYPE		char *
+#define SH_KEY_TYPE		const char *
 #define	SH_SCOPE		extern
 #define SH_RAW_ALLOCATOR	pg_malloc0
 #define SH_DECLARE
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index fd610c20a65..d77e70fbe38 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -52,7 +52,7 @@
 typedef struct manifest_file
 {
 	uint32		status;			/* hash status */
-	char	   *pathname;
+	const char *pathname;
 	size_t		size;
 	pg_checksum_type checksum_type;
 	int			checksum_length;
@@ -70,7 +70,7 @@ typedef struct manifest_file
  */
 #define SH_PREFIX		manifest_files
 #define SH_ELEMENT_TYPE	manifest_file
-#define SH_KEY_TYPE		char *
+#define SH_KEY_TYPE		const char *
 #define	SH_KEY			pathname
 #define SH_HASH_KEY(tb, key)	hash_string(key)
 #define SH_EQUAL(tb, a, b)		(strcmp(a, b) == 0)
@@ -123,7 +123,7 @@ static void verifybackup_version_cb(JsonManifestParseContext *context,
 static void verifybackup_system_identifier(JsonManifestParseContext *context,
 										   uint64 manifest_system_identifier);
 static void verifybackup_per_file_cb(JsonManifestParseContext *context,
-									 char *pathname, size_t size,
+									 const char *pathname, size_t size,
 									 pg_checksum_type checksum_type,
 									 int checksum_length,
 									 uint8 *checksum_payload);
@@ -155,7 +155,7 @@ static void report_backup_error(verifier_context *context,
 			pg_attribute_printf(2, 3);
 static void report_fatal_error(const char *pg_restrict fmt,...)
 			pg_attribute_printf(1, 2) pg_attribute_noreturn();
-static bool should_ignore_relpath(verifier_context *context, char *relpath);
+static bool should_ignore_relpath(verifier_context *context, const char *relpath);
 
 static void progress_report(bool finished);
 static void usage(void);
@@ -546,7 +546,7 @@ verifybackup_system_identifier(JsonManifestParseContext *context,
  */
 static void
 verifybackup_per_file_cb(JsonManifestParseContext *context,
-						 char *pathname, size_t size,
+						 const char *pathname, size_t size,
 						 pg_checksum_type checksum_type,
 						 int checksum_length, uint8 *checksum_payload)
 {
@@ -852,7 +852,7 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
 					 char *fullpath, uint8 *buffer)
 {
 	pg_checksum_context checksum_ctx;
-	char	   *relpath = m->pathname;
+	const char *relpath = m->pathname;
 	int			fd;
 	int			rc;
 	size_t		bytes_read = 0;
@@ -1016,13 +1016,13 @@ report_fatal_error(const char *pg_restrict fmt,...)
  * "aa/bb" is not a prefix of "aa/bbb", but it is a prefix of "aa/bb/cc".
  */
 static bool
-should_ignore_relpath(verifier_context *context, char *relpath)
+should_ignore_relpath(verifier_context *context, const char *relpath)
 {
 	SimpleStringListCell *cell;
 
 	for (cell = context->ignore_list.head; cell != NULL; cell = cell->next)
 	{
-		char	   *r = relpath;
+		const char *r = relpath;
 		char	   *v = cell->val;
 
 		while (*v != '\0' && *r == *v)
diff --git a/src/common/parse_manifest.c b/src/common/parse_manifest.c
index 373a4f6c00f..612e120b17a 100644
--- a/src/common/parse_manifest.c
+++ b/src/common/parse_manifest.c
@@ -112,7 +112,7 @@ static void json_manifest_finalize_system_identifier(JsonManifestParseState *par
 static void json_manifest_finalize_file(JsonManifestParseState *parse);
 static void json_manifest_finalize_wal_range(JsonManifestParseState *parse);
 static void verify_manifest_checksum(JsonManifestParseState *parse,
-									 char *buffer, size_t size,
+									 const char *buffer, size_t size,
 									 pg_cryptohash_ctx *incr_ctx);
 static void json_manifest_parse_failure(JsonManifestParseContext *context,
 										char *msg);
@@ -183,7 +183,7 @@ json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incs
 
 void
 json_parse_manifest_incremental_chunk(
-									  JsonManifestParseIncrementalState *incstate, char *chunk, size_t size,
+									  JsonManifestParseIncrementalState *incstate, const char *chunk, size_t size,
 									  bool is_last)
 {
 	JsonParseErrorType res,
@@ -206,7 +206,7 @@ json_parse_manifest_incremental_chunk(
 	if (!is_last)
 	{
 		if (pg_cryptohash_update(incstate->manifest_ctx,
-								 (uint8 *) chunk, size) < 0)
+								 (const uint8 *) chunk, size) < 0)
 			context->error_cb(context, "could not update checksum of manifest");
 	}
 	else
@@ -225,7 +225,7 @@ json_parse_manifest_incremental_chunk(
  * invoked and is expected not to return.
  */
 void
-json_parse_manifest(JsonManifestParseContext *context, char *buffer,
+json_parse_manifest(JsonManifestParseContext *context, const char *buffer,
 					size_t size)
 {
 	JsonLexContext *lex;
@@ -810,7 +810,7 @@ json_manifest_finalize_wal_range(JsonManifestParseState *parse)
  * parse incr_ctx will be NULL.
  */
 static void
-verify_manifest_checksum(JsonManifestParseState *parse, char *buffer,
+verify_manifest_checksum(JsonManifestParseState *parse, const char *buffer,
 						 size_t size, pg_cryptohash_ctx *incr_ctx)
 {
 	JsonManifestParseContext *context = parse->context;
@@ -858,7 +858,7 @@ verify_manifest_checksum(JsonManifestParseState *parse, char *buffer,
 	{
 		manifest_ctx = incr_ctx;
 	}
-	if (pg_cryptohash_update(manifest_ctx, (uint8 *) buffer, penultimate_newline + 1) < 0)
+	if (pg_cryptohash_update(manifest_ctx, (const uint8 *) buffer, penultimate_newline + 1) < 0)
 		context->error_cb(context, "could not update checksum of manifest");
 	if (pg_cryptohash_final(manifest_ctx, manifest_checksum_actual,
 							sizeof(manifest_checksum_actual)) < 0)
diff --git a/src/include/common/parse_manifest.h b/src/include/common/parse_manifest.h
index 2777b1e9d22..ee571a568a1 100644
--- a/src/include/common/parse_manifest.h
+++ b/src/include/common/parse_manifest.h
@@ -27,7 +27,7 @@ typedef void (*json_manifest_version_callback) (JsonManifestParseContext *,
 typedef void (*json_manifest_system_identifier_callback) (JsonManifestParseContext *,
 														  uint64 manifest_system_identifier);
 typedef void (*json_manifest_per_file_callback) (JsonManifestParseContext *,
-												 char *pathname,
+												 const char *pathname,
 												 size_t size, pg_checksum_type checksum_type,
 												 int checksum_length, uint8 *checksum_payload);
 typedef void (*json_manifest_per_wal_range_callback) (JsonManifestParseContext *,
@@ -48,10 +48,10 @@ struct JsonManifestParseContext
 };
 
 extern void json_parse_manifest(JsonManifestParseContext *context,
-								char *buffer, size_t size);
+								const char *buffer, size_t size);
 extern JsonManifestParseIncrementalState *json_parse_manifest_incremental_init(JsonManifestParseContext *context);
 extern void json_parse_manifest_incremental_chunk(
-												  JsonManifestParseIncrementalState *incstate, char *chunk, size_t size,
+												  JsonManifestParseIncrementalState *incstate, const char *chunk, size_t size,
 												  bool is_last);
 extern void json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incstate);
 
-- 
2.45.2

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#1)
Re: jsonapi type fixups

On 2024-06-18 Tu 7:48 AM, Peter Eisentraut wrote:

I have this patch series that fixes up the types of the new
incremental JSON API a bit.  Specifically, it uses "const" throughout
so that the top-level entry points such as pg_parse_json_incremental()
can declare their arguments as const char * instead of just char *. 
This just works, it doesn't require any new casting tricks.  In fact,
it removes a few unconstify() calls.

Also, a few arguments and variables that relate to object sizes should
be size_t rather than int.  At this point, this mainly makes the API
better self-documenting.  I don't think it actually works to parse
larger than 2 GB chunks (not tested).

I think this is mostly OK.

The change at line 1857 of jsonapi.c looks dubious, though. The pointer
variable p looks anything but constant. Perhaps I'm misunderstanding.

It would also be nice to reword the comment at line 3142 of jsonfuncs.c,
so it can still fit on one line.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#2)
Re: jsonapi type fixups

On 2024-06-20 Th 8:05 AM, Andrew Dunstan wrote:

On 2024-06-18 Tu 7:48 AM, Peter Eisentraut wrote:

I have this patch series that fixes up the types of the new
incremental JSON API a bit.  Specifically, it uses "const" throughout
so that the top-level entry points such as
pg_parse_json_incremental() can declare their arguments as const char
* instead of just char *.  This just works, it doesn't require any
new casting tricks.  In fact, it removes a few unconstify() calls.

Also, a few arguments and variables that relate to object sizes
should be size_t rather than int.  At this point, this mainly makes
the API better self-documenting.  I don't think it actually works to
parse larger than 2 GB chunks (not tested).

I think this is mostly OK.

The change at line 1857 of jsonapi.c looks dubious, though. The
pointer variable p looks anything but constant. Perhaps I'm
misunderstanding.

Ignore this comment, moment of brain fade. Of course it's the string
that's constant, not the pointer.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#4Peter Eisentraut
peter@eisentraut.org
In reply to: Andrew Dunstan (#2)
Re: jsonapi type fixups

On 20.06.24 14:05, Andrew Dunstan wrote:

On 2024-06-18 Tu 7:48 AM, Peter Eisentraut wrote:

I have this patch series that fixes up the types of the new
incremental JSON API a bit.  Specifically, it uses "const" throughout
so that the top-level entry points such as pg_parse_json_incremental()
can declare their arguments as const char * instead of just char *.
This just works, it doesn't require any new casting tricks.  In fact,
it removes a few unconstify() calls.

Also, a few arguments and variables that relate to object sizes should
be size_t rather than int.  At this point, this mainly makes the API
better self-documenting.  I don't think it actually works to parse
larger than 2 GB chunks (not tested).

I think this is mostly OK.

It would also be nice to reword the comment at line 3142 of jsonfuncs.c,
so it can still fit on one line.

Agreed. Committed with that fixup.