Hi all
TL;DR: formalize using StringInfo over an existing buffer as a cursor
for pq_getmsg.
In src/backend/replication/logical/worker.c we intern a pre-existing
string in a StringInfo so it can be used with the pq_getmsg functions
etc.
StringInfoData s;
....
s.data = buf;
s.len = len;
s.cursor = 0;
s.maxlen = -1;
and this strikes me as something that stringinfo.h should expose and
bless, so we don't have to fiddle with the guts of a StringInfo
directly.
Reasonable?
void
initConstantStringInfo(StringInfo str, const char *buf, Size length);
resetStringInfo() on such a string would raise an error, as would
appending and enlarging. (Right now resetting will just let you
trample on the original buffer). If anyone later wants to be able to
take such an interned stringinfo and make a writeable buffer with it,
a new function like "reallocateStringInfo" could do that, but it's
easy enough to initStringInfo a new string and appendBinaryStringInfo
it, so there's not much point.
The reason for not just copying the string when attempts to modify it
are made is clear ownership. A StringInfo usually owns its buffer, but
these ones don't, and we don't want to leave the result of a given
append call as "maybe this stringinfo still references an
outside-owned buffer, maybe it doesn't". There doesn't seem a great
deal of call for a StringInfo that can start with an external buffer
and append to it until it runs out of room, then copy it only if
needed.
Patch for constrant StringInfo attached.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 2a5f70aa69dca92e6d255b797e28cc814a39aa29 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Mon, 19 Jun 2017 12:12:20 +0800
Subject: [PATCH v1] Introduce "constant StringInfo"
Introduce constant StringInfo's, which are used as cursors over pre-existing
data by the pq_getmsg functions etc.
---
src/backend/lib/stringinfo.c | 23 +++++++++++++++
src/backend/replication/logical/worker.c | 6 +---
src/include/lib/stringinfo.h | 50 ++++++++++++++++++++++++++------
3 files changed, 65 insertions(+), 14 deletions(-)
diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c
index fd15567..6bc19ea 100644
--- a/src/backend/lib/stringinfo.c
+++ b/src/backend/lib/stringinfo.c
@@ -52,6 +52,21 @@ initStringInfo(StringInfo str)
resetStringInfo(str);
}
+void
+initConstantStringInfo(StringInfo str, const char *buf, Size length)
+{
+ str->data = buf;
+ str->len = length;
+ str->maxlen = -1;
+ str->cursor = 0;
+}
+
+bool
+stringInfoIsConstant(StringInfo str)
+{
+ return str->maxlen == -1;
+}
+
/*
* resetStringInfo
*
@@ -61,6 +76,7 @@ initStringInfo(StringInfo str)
void
resetStringInfo(StringInfo str)
{
+ Assert(!stringInfoIsConstant(str));
str->data[0] = '\0';
str->len = 0;
str->cursor = 0;
@@ -77,6 +93,7 @@ resetStringInfo(StringInfo str)
void
appendStringInfo(StringInfo str, const char *fmt,...)
{
+ Assert(!stringInfoIsConstant(str));
for (;;)
{
va_list args;
@@ -117,6 +134,7 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
size_t nprinted;
Assert(str != NULL);
+ Assert(!stringInfoIsConstant(str));
/*
* If there's hardly any space, don't bother trying, just fail to make the
@@ -169,6 +187,7 @@ void
appendStringInfoChar(StringInfo str, char ch)
{
/* Make more room if needed */
+ Assert(!stringInfoIsConstant(str));
if (str->len + 1 >= str->maxlen)
enlargeStringInfo(str, 1);
@@ -186,6 +205,7 @@ appendStringInfoChar(StringInfo str, char ch)
void
appendStringInfoSpaces(StringInfo str, int count)
{
+ Assert(!stringInfoIsConstant(str));
if (count > 0)
{
/* Make more room if needed */
@@ -208,6 +228,7 @@ void
appendBinaryStringInfo(StringInfo str, const char *data, int datalen)
{
Assert(str != NULL);
+ Assert(!stringInfoIsConstant(str));
/* Make more room if needed */
enlargeStringInfo(str, datalen);
@@ -246,6 +267,8 @@ enlargeStringInfo(StringInfo str, int needed)
{
int newlen;
+ Assert(!stringInfoIsConstant(str));
+
/*
* Guard against out-of-range "needed" values. Without this, we can get
* an overflow or infinite loop in the following.
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index c67720b..4160a7f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1049,11 +1049,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
/* Ensure we are reading the data into our memory context. */
MemoryContextSwitchTo(ApplyMessageContext);
- s.data = buf;
- s.len = len;
- s.cursor = 0;
- s.maxlen = -1;
-
+ initConstantStringInfo(&s, buf, len);
c = pq_getmsgbyte(&s);
if (c == 'w')
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 316c196..ff50a04 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -21,15 +21,17 @@
* StringInfoData holds information about an extensible string.
* data is the current buffer for the string (allocated with palloc).
* len is the current string length. There is guaranteed to be
- * a terminating '\0' at data[len], although this is not very
- * useful when the string holds binary data rather than text.
+ * a terminating '\0' at data[len] unless the StringInfo was
+ * constructed from a caller-supplied buffer.
* maxlen is the allocated size in bytes of 'data', i.e. the maximum
* string size (including the terminating '\0' char) that we can
* currently store in 'data' without having to reallocate
- * more space. We must always have maxlen > len.
- * cursor is initialized to zero by makeStringInfo or initStringInfo,
- * but is not otherwise touched by the stringinfo.c routines.
- * Some routines use it to scan through a StringInfo.
+ * more space. We must always have maxlen > len unless the
+ * StringInfo is constant, in which case maxlen is -1.
+ * cursor is initialized to zero by makeStringInfo, initStringInfo
+ * and initConstantStringInfo, but is not otherwise touched by the
+ * stringinfo.c routines. Some routines use it to scan through a
+ * StringInfo.
*-------------------------
*/
typedef struct StringInfoData
@@ -44,7 +46,7 @@ typedef StringInfoData *StringInfo;
/*------------------------
- * There are two ways to create a StringInfo object initially:
+ * There are three ways to create a StringInfo object initially:
*
* StringInfo stringptr = makeStringInfo();
* Both the StringInfoData and the data buffer are palloc'd.
@@ -55,8 +57,21 @@ typedef StringInfoData *StringInfo;
* This is the easiest approach for a StringInfo object that will
* only live as long as the current routine.
*
- * To destroy a StringInfo, pfree() the data buffer, and then pfree() the
- * StringInfoData if it was palloc'd. There's no special support for this.
+ * char * buffer = ...;
+ * Size buffer_length = ....;
+ * StringInfoData string;
+ * initConstantStringInfo(&string, buffer, buffer_length);
+ * The data buffer is a constant that remains owned by the caller.
+ * Resetting this StringInfo or appending to it is not allowed.
+ * This form is mainly useful when the StringInfo is used as a
+ * cursor over pre-existing data.
+ *
+ * To destroy a non-constant StringInfo, pfree() the data buffer, and then
+ * pfree() the StringInfoData if it was palloc'd. There's no special support
+ * for this. It is usually sufficient to just let the buffer be cleaned up
+ * along with the rest of the memory context in which it was allocated.
+ *
+ * There is no need to destroy a constant StringInfo.
*
* NOTE: some routines build up a string using StringInfo, and then
* release the StringInfoData but return the data string itself to their
@@ -79,6 +94,16 @@ extern StringInfo makeStringInfo(void);
extern void initStringInfo(StringInfo str);
/*------------------------
+ * initConstantStringInfo
+ * Initialize a StringInfoData struct (with previously undefined contents) to
+ * wrap the supplied buffer and length. The passed buffer remains owned by the
+ * caller. The resulting StringInfo is constant and cannot be appended to or
+ * resized, but provides a cursor over the buffer.
+ */
+extern void
+initConstantStringInfo(StringInfo str, const char *buf, Size length);
+
+/*------------------------
* resetStringInfo
* Clears the current content of the StringInfo, if any. The
* StringInfo remains valid.
@@ -149,4 +174,11 @@ extern void appendBinaryStringInfo(StringInfo str,
*/
extern void enlargeStringInfo(StringInfo str, int needed);
+/*------------------------
+ * stringInfoIsConstant
+ * Returns true if the StringInfo points to an externally allocated constant
+ * buffer per initConstantStringInfo(...)
+ */
+extern bool stringInfoIsConstant(StringInfo str);
+
#endif /* STRINGINFO_H */
--
2.9.4
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
