Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f0efa5a

Browse filesBrowse files
david-rowleydavid-rowley
committed
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 parent 01575ad commit f0efa5a
Copy full SHA for f0efa5a

File tree

9 files changed

+127
-83
lines changed
Filter options

9 files changed

+127
-83
lines changed

‎src/backend/replication/logical/applyparallelworker.c

Copy file name to clipboardExpand all lines: src/backend/replication/logical/applyparallelworker.c
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,10 +774,7 @@ LogicalParallelApplyLoop(shm_mq_handle *mqh)
774774
if (len == 0)
775775
elog(ERROR, "invalid message length");
776776

777-
s.cursor = 0;
778-
s.maxlen = -1;
779-
s.data = (char *) data;
780-
s.len = len;
777+
initReadOnlyStringInfo(&s, data, len);
781778

782779
/*
783780
* The first byte of messages sent from leader apply worker to

‎src/backend/replication/logical/proto.c

Copy file name to clipboardExpand all lines: src/backend/replication/logical/proto.c
+9-9Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,7 @@ logicalrep_read_tuple(StringInfo in, LogicalRepTupleData *tuple)
879879
/* Read the data */
880880
for (i = 0; i < natts; i++)
881881
{
882+
char *buff;
882883
char kind;
883884
int len;
884885
StringInfo value = &tuple->colvalues[i];
@@ -899,19 +900,18 @@ logicalrep_read_tuple(StringInfo in, LogicalRepTupleData *tuple)
899900
len = pq_getmsgint(in, 4); /* read length */
900901

901902
/* and data */
902-
value->data = palloc(len + 1);
903-
pq_copymsgbytes(in, value->data, len);
903+
buff = palloc(len + 1);
904+
pq_copymsgbytes(in, buff, len);
904905

905906
/*
906-
* Not strictly necessary for LOGICALREP_COLUMN_BINARY, but
907-
* per StringInfo practice.
907+
* NUL termination is required for LOGICALREP_COLUMN_TEXT mode
908+
* as input functions require that. For
909+
* LOGICALREP_COLUMN_BINARY it's not technically required, but
910+
* it's harmless.
908911
*/
909-
value->data[len] = '\0';
912+
buff[len] = '\0';
910913

911-
/* make StringInfo fully valid */
912-
value->len = len;
913-
value->cursor = 0;
914-
value->maxlen = len;
914+
initStringInfoFromString(value, buff, len);
915915
break;
916916
default:
917917
elog(ERROR, "unrecognized data representation type '%c'", kind);

‎src/backend/replication/logical/worker.c

Copy file name to clipboardExpand all lines: src/backend/replication/logical/worker.c
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3582,10 +3582,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
35823582
/* Ensure we are reading the data into our memory context. */
35833583
MemoryContextSwitchTo(ApplyMessageContext);
35843584

3585-
s.data = buf;
3586-
s.len = len;
3587-
s.cursor = 0;
3588-
s.maxlen = -1;
3585+
initReadOnlyStringInfo(&s, buf, len);
35893586

35903587
c = pq_getmsgbyte(&s);
35913588

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+9-13Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,23 +1817,19 @@ exec_bind_message(StringInfo input_message)
18171817

18181818
if (!isNull)
18191819
{
1820-
const char *pvalue = pq_getmsgbytes(input_message, plength);
1820+
char *pvalue;
18211821

18221822
/*
1823-
* Rather than copying data around, we just set up a phony
1823+
* Rather than copying data around, we just initialize a
18241824
* StringInfo pointing to the correct portion of the message
1825-
* buffer. We assume we can scribble on the message buffer so
1826-
* as to maintain the convention that StringInfos have a
1827-
* trailing null. This is grotty but is a big win when
1828-
* dealing with very large parameter strings.
1825+
* buffer. We assume we can scribble on the message buffer to
1826+
* add a trailing NUL which is required for the input function
1827+
* call.
18291828
*/
1830-
pbuf.data = unconstify(char *, pvalue);
1831-
pbuf.maxlen = plength + 1;
1832-
pbuf.len = plength;
1833-
pbuf.cursor = 0;
1834-
1835-
csave = pbuf.data[plength];
1836-
pbuf.data[plength] = '\0';
1829+
pvalue = unconstify(char *, pq_getmsgbytes(input_message, plength));
1830+
csave = pvalue[plength];
1831+
pvalue[plength] = '\0';
1832+
initReadOnlyStringInfo(&pbuf, pvalue, plength);
18371833
}
18381834
else
18391835
{

‎src/backend/utils/adt/array_userfuncs.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/array_userfuncs.c
+4-14Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,6 @@ array_agg_deserialize(PG_FUNCTION_ARGS)
784784
{
785785
int itemlen;
786786
StringInfoData elem_buf;
787-
char csave;
788787

789788
if (result->dnulls[i])
790789
{
@@ -799,28 +798,19 @@ array_agg_deserialize(PG_FUNCTION_ARGS)
799798
errmsg("insufficient data left in message")));
800799

801800
/*
802-
* Rather than copying data around, we just set up a phony
803-
* StringInfo pointing to the correct portion of the input buffer.
804-
* We assume we can scribble on the input buffer so as to maintain
805-
* the convention that StringInfos have a trailing null.
801+
* Rather than copying data around, we just initialize a
802+
* StringInfo pointing to the correct portion of the message
803+
* buffer.
806804
*/
807-
elem_buf.data = &buf.data[buf.cursor];
808-
elem_buf.maxlen = itemlen + 1;
809-
elem_buf.len = itemlen;
810-
elem_buf.cursor = 0;
805+
initReadOnlyStringInfo(&elem_buf, &buf.data[buf.cursor], itemlen);
811806

812807
buf.cursor += itemlen;
813808

814-
csave = buf.data[buf.cursor];
815-
buf.data[buf.cursor] = '\0';
816-
817809
/* Now call the element's receiveproc */
818810
result->dvalues[i] = ReceiveFunctionCall(&iodata->typreceive,
819811
&elem_buf,
820812
iodata->typioparam,
821813
-1);
822-
823-
buf.data[buf.cursor] = csave;
824814
}
825815
}
826816

‎src/backend/utils/adt/arrayfuncs.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/arrayfuncs.c
+3-14Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,6 @@ ReadArrayBinary(StringInfo buf,
14751475
{
14761476
int itemlen;
14771477
StringInfoData elem_buf;
1478-
char csave;
14791478

14801479
/* Get and check the item length */
14811480
itemlen = pq_getmsgint(buf, 4);
@@ -1494,21 +1493,13 @@ ReadArrayBinary(StringInfo buf,
14941493
}
14951494

14961495
/*
1497-
* Rather than copying data around, we just set up a phony StringInfo
1498-
* pointing to the correct portion of the input buffer. We assume we
1499-
* can scribble on the input buffer so as to maintain the convention
1500-
* that StringInfos have a trailing null.
1496+
* Rather than copying data around, we just initialize a StringInfo
1497+
* pointing to the correct portion of the message buffer.
15011498
*/
1502-
elem_buf.data = &buf->data[buf->cursor];
1503-
elem_buf.maxlen = itemlen + 1;
1504-
elem_buf.len = itemlen;
1505-
elem_buf.cursor = 0;
1499+
initReadOnlyStringInfo(&elem_buf, &buf->data[buf->cursor], itemlen);
15061500

15071501
buf->cursor += itemlen;
15081502

1509-
csave = buf->data[buf->cursor];
1510-
buf->data[buf->cursor] = '\0';
1511-
15121503
/* Now call the element's receiveproc */
15131504
values[i] = ReceiveFunctionCall(receiveproc, &elem_buf,
15141505
typioparam, typmod);
@@ -1520,8 +1511,6 @@ ReadArrayBinary(StringInfo buf,
15201511
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
15211512
errmsg("improper binary format in array element %d",
15221513
i + 1)));
1523-
1524-
buf->data[buf->cursor] = csave;
15251514
}
15261515

15271516
/*

‎src/backend/utils/adt/rowtypes.c

Copy file name to clipboardExpand all lines: src/backend/utils/adt/rowtypes.c
+7-16Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,6 @@ record_recv(PG_FUNCTION_ARGS)
569569
int itemlen;
570570
StringInfoData item_buf;
571571
StringInfo bufptr;
572-
char csave;
573572

574573
/* Ignore dropped columns in datatype, but fill with nulls */
575574
if (att->attisdropped)
@@ -619,25 +618,19 @@ record_recv(PG_FUNCTION_ARGS)
619618
/* -1 length means NULL */
620619
bufptr = NULL;
621620
nulls[i] = true;
622-
csave = 0; /* keep compiler quiet */
623621
}
624622
else
625623
{
624+
char *strbuff;
625+
626626
/*
627-
* Rather than copying data around, we just set up a phony
628-
* StringInfo pointing to the correct portion of the input buffer.
629-
* We assume we can scribble on the input buffer so as to maintain
630-
* the convention that StringInfos have a trailing null.
627+
* Rather than copying data around, we just initialize a
628+
* StringInfo pointing to the correct portion of the message
629+
* buffer.
631630
*/
632-
item_buf.data = &buf->data[buf->cursor];
633-
item_buf.maxlen = itemlen + 1;
634-
item_buf.len = itemlen;
635-
item_buf.cursor = 0;
636-
631+
strbuff = &buf->data[buf->cursor];
637632
buf->cursor += itemlen;
638-
639-
csave = buf->data[buf->cursor];
640-
buf->data[buf->cursor] = '\0';
633+
initReadOnlyStringInfo(&item_buf, strbuff, itemlen);
641634

642635
bufptr = &item_buf;
643636
nulls[i] = false;
@@ -667,8 +660,6 @@ record_recv(PG_FUNCTION_ARGS)
667660
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
668661
errmsg("improper binary format in record column %d",
669662
i + 1)));
670-
671-
buf->data[buf->cursor] = csave;
672663
}
673664
}
674665

‎src/common/stringinfo.c

Copy file name to clipboardExpand all lines: src/common/stringinfo.c
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,16 @@ initStringInfo(StringInfo str)
7070
*
7171
* Reset the StringInfo: the data buffer remains valid, but its
7272
* previous content, if any, is cleared.
73+
*
74+
* Read-only StringInfos as initialized by initReadOnlyStringInfo cannot be
75+
* reset.
7376
*/
7477
void
7578
resetStringInfo(StringInfo str)
7679
{
80+
/* don't allow resets of read-only StringInfos */
81+
Assert(str->maxlen != 0);
82+
7783
str->data[0] = '\0';
7884
str->len = 0;
7985
str->cursor = 0;
@@ -284,6 +290,9 @@ enlargeStringInfo(StringInfo str, int needed)
284290
{
285291
int newlen;
286292

293+
/* validate this is not a read-only StringInfo */
294+
Assert(str->maxlen != 0);
295+
287296
/*
288297
* Guard against out-of-range "needed" values. Without this, we can get
289298
* an overflow or infinite loop in the following.

‎src/include/lib/stringinfo.h

Copy file name to clipboardExpand all lines: src/include/lib/stringinfo.h
+84-9Lines changed: 84 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,27 @@
2020

2121
/*-------------------------
2222
* StringInfoData holds information about an extensible string.
23-
* data is the current buffer for the string (allocated with palloc).
24-
* len is the current string length. There is guaranteed to be
25-
* a terminating '\0' at data[len], although this is not very
26-
* useful when the string holds binary data rather than text.
23+
* data is the current buffer for the string.
24+
* len is the current string length. Except in the case of read-only
25+
* strings described below, there is guaranteed to be a
26+
* terminating '\0' at data[len].
2727
* maxlen is the allocated size in bytes of 'data', i.e. the maximum
2828
* string size (including the terminating '\0' char) that we can
2929
* currently store in 'data' without having to reallocate
30-
* more space. We must always have maxlen > len.
31-
* cursor is initialized to zero by makeStringInfo or initStringInfo,
32-
* but is not otherwise touched by the stringinfo.c routines.
33-
* Some routines use it to scan through a StringInfo.
30+
* more space. We must always have maxlen > len, except
31+
* in the read-only case described below.
32+
* cursor is initialized to zero by makeStringInfo, initStringInfo,
33+
* initReadOnlyStringInfo and initStringInfoFromString but is not
34+
* otherwise touched by the stringinfo.c routines. Some routines
35+
* use it to scan through a StringInfo.
36+
*
37+
* As a special case, a StringInfoData can be initialized with a read-only
38+
* string buffer. In this case "data" does not necessarily point at a
39+
* palloc'd chunk, and management of the buffer storage is the caller's
40+
* responsibility. maxlen is set to zero to indicate that this is the case.
41+
* Read-only StringInfoDatas cannot be appended to or reset.
42+
* Also, it is caller's option whether a read-only string buffer has a
43+
* terminating '\0' or not. This depends on the intended usage.
3444
*-------------------------
3545
*/
3646
typedef struct StringInfoData
@@ -45,7 +55,7 @@ typedef StringInfoData *StringInfo;
4555

4656

4757
/*------------------------
48-
* There are two ways to create a StringInfo object initially:
58+
* There are four ways to create a StringInfo object initially:
4959
*
5060
* StringInfo stringptr = makeStringInfo();
5161
* Both the StringInfoData and the data buffer are palloc'd.
@@ -56,8 +66,31 @@ typedef StringInfoData *StringInfo;
5666
* This is the easiest approach for a StringInfo object that will
5767
* only live as long as the current routine.
5868
*
69+
* StringInfoData string;
70+
* initReadOnlyStringInfo(&string, existingbuf, len);
71+
* The StringInfoData's data field is set to point directly to the
72+
* existing buffer and the StringInfoData's len is set to the given len.
73+
* The given buffer can point to memory that's not managed by palloc or
74+
* is pointing partway through a palloc'd chunk. The maxlen field is set
75+
* to 0. A read-only StringInfo cannot be appended to using any of the
76+
* appendStringInfo functions or reset with resetStringInfo(). The given
77+
* buffer can optionally omit the trailing NUL.
78+
*
79+
* StringInfoData string;
80+
* initStringInfoFromString(&string, palloced_buf, len);
81+
* The StringInfoData's data field is set to point directly to the given
82+
* buffer and the StringInfoData's len is set to the given len. This
83+
* method of initialization is useful when the buffer already exists.
84+
* StringInfos initialized this way can be appended to using the
85+
* appendStringInfo functions and reset with resetStringInfo(). The
86+
* given buffer must be NUL-terminated. The palloc'd buffer is assumed
87+
* to be len + 1 in size.
88+
*
5989
* To destroy a StringInfo, pfree() the data buffer, and then pfree() the
6090
* StringInfoData if it was palloc'd. There's no special support for this.
91+
* However, if the StringInfo was initialized using initReadOnlyStringInfo()
92+
* then the caller will need to consider if it is safe to pfree the data
93+
* buffer.
6194
*
6295
* NOTE: some routines build up a string using StringInfo, and then
6396
* release the StringInfoData but return the data string itself to their
@@ -79,6 +112,48 @@ extern StringInfo makeStringInfo(void);
79112
*/
80113
extern void initStringInfo(StringInfo str);
81114

115+
/*------------------------
116+
* initReadOnlyStringInfo
117+
* Initialize a StringInfoData struct from an existing string without copying
118+
* the string. The caller is responsible for ensuring the given string
119+
* remains valid as long as the StringInfoData does. Calls to this are used
120+
* in performance critical locations where allocating a new buffer and copying
121+
* would be too costly. Read-only StringInfoData's may not be appended to
122+
* using any of the appendStringInfo functions or reset with
123+
* resetStringInfo().
124+
*
125+
* 'data' does not need to point directly to a palloc'd chunk of memory and may
126+
* omit the NUL termination character at data[len].
127+
*/
128+
static inline void
129+
initReadOnlyStringInfo(StringInfo str, char *data, int len)
130+
{
131+
str->data = data;
132+
str->len = len;
133+
str->maxlen = 0; /* read-only */
134+
str->cursor = 0;
135+
}
136+
137+
/*------------------------
138+
* initStringInfoFromString
139+
* Initialize a StringInfoData struct from an existing string without copying
140+
* the string. 'data' must be a valid palloc'd chunk of memory that can have
141+
* repalloc() called should more space be required during a call to any of the
142+
* appendStringInfo functions.
143+
*
144+
* 'data' must be NUL terminated at 'len' bytes.
145+
*/
146+
static inline void
147+
initStringInfoFromString(StringInfo str, char *data, int len)
148+
{
149+
Assert(data[len] == '\0');
150+
151+
str->data = data;
152+
str->len = len;
153+
str->maxlen = len + 1;
154+
str->cursor = 0;
155+
}
156+
82157
/*------------------------
83158
* resetStringInfo
84159
* Clears the current content of the StringInfo, if any. The

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.