Introduce the concept of read-only StringInfos

There were various places in our codebase which conjured up a StringInfo
by manually assigning the StringInfo fields and setting the data field
to point to some existing buffer.  There wasn't much consistency here as
to what fields like maxlen got set to and in one location we didn't
correctly ensure that the buffer was correctly NUL terminated at len
bytes, as per what was documented as required in stringinfo.h

Here we introduce 2 new functions to initialize StringInfos.  One allows
callers to initialize a StringInfo passing along a buffer that is
already allocated by palloc.  Here the StringInfo code uses this buffer
directly rather than doing any memcpying into a new allocation.  Having
this as a function allows us to verify the buffer is correctly NUL
terminated.  StringInfos initialized this way can be appended to and
reset just like any other normal StringInfo.

The other new initialization function also accepts an existing buffer,
but the given buffer does not need to be a pointer to a palloc'd chunk.
This buffer could be a pointer pointing partway into some palloc'd chunk
or may not even be palloc'd at all.  StringInfos initialized this way
are deemed as "read-only".  This means that it's not possible to
append to them or reset them.

For the latter of the two new initialization functions mentioned above,
we relax the requirement that the data buffer must be NUL terminated.
Relaxing this requirement is convenient in a few places as it can save
us from having to allocate an entire new buffer just to add the NUL
terminator or save us from having to temporarily add a NUL only to have to
put the original char back again later.

Incompatibility note:

Here we also forego adding the NUL in a few places where it does not
seem to be required.  These locations are passing the given StringInfo
into a type's receive function.  It does not seem like any of our
built-in receive functions require this, but perhaps there's some UDT
out there in the wild which does require this.  It is likely worthy of
a mention in the release notes that a UDT's receive function mustn't rely
on the input StringInfo being NUL terminated.

Author: David Rowley
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAApHDvorfO3iBZ%3DxpiZvp3uHtJVLyFaPBSvcAhAq2HPLnaNSwQ%40mail.gmail.com

1 files changed, 3 insertions, 14 deletions

diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 7828a6264b5..6a920a02b72 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1475,7 +1475,6 @@ ReadArrayBinary(StringInfo buf,
 	{
 		int			itemlen;
 		StringInfoData elem_buf;
-		char		csave;
 
 		/* Get and check the item length */
 		itemlen = pq_getmsgint(buf, 4);
@@ -1494,21 +1493,13 @@ ReadArrayBinary(StringInfo buf,
 		}
 
 		/*
-		 * Rather than copying data around, we just set up a phony StringInfo
-		 * pointing to the correct portion of the input buffer. We assume we
-		 * can scribble on the input buffer so as to maintain the convention
-		 * that StringInfos have a trailing null.
+		 * Rather than copying data around, we just initialize a StringInfo
+		 * pointing to the correct portion of the message buffer.
 		 */
-		elem_buf.data = &buf->data[buf->cursor];
-		elem_buf.maxlen = itemlen + 1;
-		elem_buf.len = itemlen;
-		elem_buf.cursor = 0;
+		initReadOnlyStringInfo(&elem_buf, &buf->data[buf->cursor], itemlen);
 
 		buf->cursor += itemlen;
 
-		csave = buf->data[buf->cursor];
-		buf->data[buf->cursor] = '\0';
-
 		/* Now call the element's receiveproc */
 		values[i] = ReceiveFunctionCall(receiveproc, &elem_buf,
 										typioparam, typmod);
@@ -1520,8 +1511,6 @@ ReadArrayBinary(StringInfo buf,
 					(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
 					 errmsg("improper binary format in array element %d",
 							i + 1)));
-
-		buf->data[buf->cursor] = csave;
 	}
 
 	/*