Make some xlogreader messages more accurate

From 24876cad15d43fde310ad12a801aec09c35e18a5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 2 Mar 2023 08:54:22 +0100
Subject: [PATCH v1] Improve various error messages

Update error messages "{invalid|incorrect|wrong} {length|size}" to be
more precise by using the appropriate adjective and adding more detail
if available.
---
 src/backend/access/transam/twophase.c       | 10 ++++------
 src/backend/access/transam/xlogreader.c     |  2 +-
 src/backend/access/transam/xlogrecovery.c   |  5 ++++-
 src/backend/postmaster/postmaster.c         |  8 ++++++--
 src/backend/replication/logical/worker.c    |  2 +-
 src/backend/statistics/mcv.c                | 19 +++++++------------
 src/backend/utils/adt/network.c             |  6 ++++--
 src/backend/utils/adt/tsquery.c             |  2 +-
 src/backend/utils/adt/tsvector.c            |  2 +-
 src/backend/utils/adt/varbit.c              |  6 ++++--
 src/pl/plpython/expected/plpython_types.out |  2 +-
 src/pl/plpython/plpy_typeio.c               |  2 +-
 src/test/modules/test_rbtree/test_rbtree.c  |  2 +-
 13 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 068e59bec0..e644a0c4d3 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1319,10 +1319,8 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
 		stat.st_size > MaxAllocSize)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATA_CORRUPTED),
-				 errmsg_plural("incorrect size of file \"%s\": %lld byte",
-							   "incorrect size of file \"%s\": %lld bytes",
-							   (long long int) stat.st_size, path,
-							   (long long int) stat.st_size)));
+				 errmsg("size of file \"%s\" (%lld) out of valid range",
+						path, (long long int) stat.st_size)));
 
 	crc_offset = stat.st_size - sizeof(pg_crc32c);
 	if (crc_offset != MAXALIGN(crc_offset))
@@ -1367,8 +1365,8 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
 	if (hdr->total_len != stat.st_size)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATA_CORRUPTED),
-				 errmsg("invalid size stored in file \"%s\"",
-						path)));
+				 errmsg("incorrect size stored in file \"%s\"", path),
+				 errdetail("Expected %lld, got %u.", (long long int) stat.st_size, hdr->total_len)));
 
 	INIT_CRC32C(calc_crc);
 	COMP_CRC32C(calc_crc, buf, crc_offset);
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index aa6c929477..f431a84fbc 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1942,7 +1942,7 @@ DecodeXLogRecord(XLogReaderState *state,
 
 shortdata_err:
 	report_invalid_record(state,
-						  "record with invalid length at %X/%X",
+						  "record with incorrect length at %X/%X",
 						  LSN_FORMAT_ARGS(state->ReadRecPtr));
 err:
 	*errormsg = state->errormsg_buf;
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index dbe9394762..7d5b8b9775 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -3984,7 +3984,10 @@ ReadCheckpointRecord(XLogPrefetcher *xlogprefetcher, XLogRecPtr RecPtr,
 	if (record->xl_tot_len != SizeOfXLogRecord + SizeOfXLogRecordDataHeaderShort + sizeof(CheckPoint))
 	{
 		ereport(LOG,
-				(errmsg("invalid length of checkpoint record")));
+				(errmsg("incorrect length of checkpoint record"),
+				 errdetail("Expected %zu, got %u.",
+						   SizeOfXLogRecord + SizeOfXLogRecordDataHeaderShort + sizeof(CheckPoint),
+						   record->xl_tot_len)));
 		return NULL;
 	}
 	return record;
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 2552327d90..1080a72c8d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1993,7 +1993,9 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 	{
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("invalid length of startup packet")));
+				 errmsg("invalid length of startup packet"),
+				 errdetail("Expected %d..%d, got %d.",
+						   (int32) sizeof(ProtocolVersion), MAX_STARTUP_PACKET_LENGTH, len)));
 		return STATUS_ERROR;
 	}
 
@@ -2026,7 +2028,9 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 		{
 			ereport(COMMERROR,
 					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("invalid length of startup packet")));
+					 errmsg("incorrect length of cancel request packet"),
+					 errdetail("Expected %zu, got %d.",
+							   sizeof(CancelRequestPacket), len)));
 			return STATUS_ERROR;
 		}
 		processCancelRequest(port, buf);
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index cfb2ab6248..62b3fb56f6 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2090,7 +2090,7 @@ apply_spooled_messages(FileSet *stream_fileset, TransactionId xid,
 
 		/* do we have a correct length? */
 		if (len <= 0)
-			elog(ERROR, "incorrect length %d in streaming transaction's changes file \"%s\"",
+			elog(ERROR, "invalid length %d in streaming transaction's changes file \"%s\"",
 				 len, path);
 
 		/* make sure we have sufficiently large buffer */
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 03b9f04bb5..5590f78c6d 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1064,18 +1064,13 @@ statext_mcv_deserialize(bytea *data)
 		elog(ERROR, "invalid MCV type %u (expected %u)",
 			 mcvlist->type, STATS_MCV_TYPE_BASIC);
 
-	if (mcvlist->ndimensions == 0)
-		elog(ERROR, "invalid zero-length dimension array in MCVList");
-	else if ((mcvlist->ndimensions > STATS_MAX_DIMENSIONS) ||
-			 (mcvlist->ndimensions < 0))
-		elog(ERROR, "invalid length (%d) dimension array in MCVList",
-			 mcvlist->ndimensions);
-
-	if (mcvlist->nitems == 0)
-		elog(ERROR, "invalid zero-length item array in MCVList");
-	else if (mcvlist->nitems > STATS_MCVLIST_MAX_ITEMS)
-		elog(ERROR, "invalid length (%u) item array in MCVList",
-			 mcvlist->nitems);
+	if (mcvlist->ndimensions < 1 ||	mcvlist->ndimensions > STATS_MAX_DIMENSIONS)
+		elog(ERROR, "length of dimension array in MCVList (%d) out of valid range (%d..%d)",
+			 mcvlist->ndimensions, 1, STATS_MAX_DIMENSIONS);
+
+	if (mcvlist->nitems < 1 || mcvlist->nitems > STATS_MCVLIST_MAX_ITEMS)
+		elog(ERROR, "length of item array in MCVList (%u) out of valid range (%d..%d)",
+			 mcvlist->nitems, 1, STATS_MCVLIST_MAX_ITEMS);
 
 	nitems = mcvlist->nitems;
 	ndims = mcvlist->ndimensions;
diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index 42a4d9d44e..361e6e19af 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -222,8 +222,10 @@ network_recv(StringInfo buf, bool is_cidr)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
 		/* translator: %s is inet or cidr */
-				 errmsg("invalid length in external \"%s\" value",
-						is_cidr ? "cidr" : "inet")));
+				 errmsg("incorrect length in external \"%s\" value",
+						is_cidr ? "cidr" : "inet"),
+				 errdetail("Expected %d, got %d.",
+						   ip_addrsize(addr), nb)));
 
 	addrptr = (char *) ip_addr(addr);
 	for (i = 0; i < nb; i++)
diff --git a/src/backend/utils/adt/tsquery.c b/src/backend/utils/adt/tsquery.c
index 67ad876a27..a6fa770c95 100644
--- a/src/backend/utils/adt/tsquery.c
+++ b/src/backend/utils/adt/tsquery.c
@@ -1242,7 +1242,7 @@ tsqueryrecv(PG_FUNCTION_ARGS)
 
 	size = pq_getmsgint(buf, sizeof(uint32));
 	if (size > (MaxAllocSize / sizeof(QueryItem)))
-		elog(ERROR, "invalid size of tsquery");
+		elog(ERROR, "size of tsquery (%u) out of valid range", size);
 
 	/* Allocate space to temporarily hold operand strings */
 	operands = palloc(size * sizeof(char *));
diff --git a/src/backend/utils/adt/tsvector.c b/src/backend/utils/adt/tsvector.c
index 0e66f362c3..c0ce9282b9 100644
--- a/src/backend/utils/adt/tsvector.c
+++ b/src/backend/utils/adt/tsvector.c
@@ -462,7 +462,7 @@ tsvectorrecv(PG_FUNCTION_ARGS)
 
 	nentries = pq_getmsgint(buf, sizeof(int32));
 	if (nentries < 0 || nentries > (MaxAllocSize / sizeof(WordEntry)))
-		elog(ERROR, "invalid size of tsvector");
+		elog(ERROR, "size of tsvector (%d) out of valid range", nentries);
 
 	hdrlen = DATAHDRSIZE + sizeof(WordEntry) * nentries;
 
diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c
index 3dbbd1207f..2da5d6ebea 100644
--- a/src/backend/utils/adt/varbit.c
+++ b/src/backend/utils/adt/varbit.c
@@ -344,7 +344,8 @@ bit_recv(PG_FUNCTION_ARGS)
 	if (bitlen < 0 || bitlen > VARBITMAXLEN)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
-				 errmsg("invalid length in external bit string")));
+				 errmsg("length in external bit string (%d) out of valid range (%d..%d)",
+						bitlen, 0, VARBITMAXLEN)));
 
 	/*
 	 * Sometimes atttypmod is not supplied. If it is supplied we need to make
@@ -649,7 +650,8 @@ varbit_recv(PG_FUNCTION_ARGS)
 	if (bitlen < 0 || bitlen > VARBITMAXLEN)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
-				 errmsg("invalid length in external bit string")));
+				 errmsg("length in external bit string (%d) out of valid range (%d..%d)",
+						bitlen, 0, VARBITMAXLEN)));
 
 	/*
 	 * Sometimes atttypmod is not supplied. If it is supplied we need to make
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index a470911c2e..fc451d86b4 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -691,7 +691,7 @@ CREATE FUNCTION test_type_conversion_mdarray_malformed() RETURNS int[] AS $$
 return [[1,2,3],[4,5]]
 $$ LANGUAGE plpython3u;
 SELECT * FROM test_type_conversion_mdarray_malformed();
-ERROR:  wrong length of inner sequence: has length 2, but 3 was expected
+ERROR:  incorrect length of inner sequence: has length 2, but 3 was expected
 DETAIL:  To construct a multidimensional array, the inner sequences must all have the same length.
 CONTEXT:  while creating return value
 PL/Python function "test_type_conversion_mdarray_malformed"
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 7018c9d404..66850a00cb 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -1258,7 +1258,7 @@ PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject *list,
 	if (PySequence_Length(list) != dims[dim])
 		ereport(ERROR,
 				(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
-				 errmsg("wrong length of inner sequence: has length %d, but %d was expected",
+				 errmsg("incorrect length of inner sequence: has length %d, but %d was expected",
 						(int) PySequence_Length(list), dims[dim]),
 				 (errdetail("To construct a multidimensional array, the inner sequences must all have the same length."))));
 
diff --git a/src/test/modules/test_rbtree/test_rbtree.c b/src/test/modules/test_rbtree/test_rbtree.c
index e4eb154378..99c288571a 100644
--- a/src/test/modules/test_rbtree/test_rbtree.c
+++ b/src/test/modules/test_rbtree/test_rbtree.c
@@ -505,7 +505,7 @@ test_rb_tree(PG_FUNCTION_ARGS)
 	int			size = PG_GETARG_INT32(0);
 
 	if (size <= 0 || size > MaxAllocSize / sizeof(int))
-		elog(ERROR, "invalid size for test_rb_tree: %d", size);
+		elog(ERROR, "size for test_rb_tree out of valid range: %d", size);
 	testleftright(size);
 	testrightleft(size);
 	testfind(size);
-- 
2.39.2