v1-0001-Adjust-code-to-highlight-appendStringInfo-misusag.patch

From 014d8c4d090e9267deaaa8460c7d9c615c655d5f Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Thu, 15 Sep 2022 21:27:00 +1200
Subject: [PATCH v1 1/3] Adjust code to highlight appendStringInfo misusages

Not intended for commit
---
 src/backend/nodes/outfuncs.c            | 40 ++++++++++++-------------
 src/backend/utils/adt/xml.c             |  2 +-
 src/bin/psql/create_help.pl             |  7 ++++-
 src/common/stringinfo.c                 |  6 ++--
 src/fe_utils/string_utils.c             |  4 +--
 src/include/lib/stringinfo.h            | 28 +++++++++++++++--
 src/interfaces/libpq-oauth/oauth-curl.c |  6 ++--
 src/interfaces/libpq/exports.txt        |  4 +--
 src/interfaces/libpq/pqexpbuffer.c      |  6 ++--
 src/interfaces/libpq/pqexpbuffer.h      | 26 ++++++++++++++--
 10 files changed, 89 insertions(+), 40 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 953c5797c5d..4b1e7675fc9 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -45,11 +45,11 @@ static void outDouble(StringInfo str, double d);
 
 /* Write an integer field (anything written as ":fldname %d") */
 #define WRITE_INT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", node->fldname)
 
 /* Write an unsigned integer field (anything written as ":fldname %u") */
 #define WRITE_UINT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %u", node->fldname)
 
 /* Write a signed integer field (anything written with INT64_FORMAT) */
 #define WRITE_INT64_FIELD(fldname) \
@@ -59,84 +59,84 @@ static void outDouble(StringInfo str, double d);
 
 /* Write an unsigned integer field (anything written with UINT64_FORMAT) */
 #define WRITE_UINT64_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
 					 node->fldname)
 
 /* Write an OID field (don't hard-wire assumption that OID is same as uint) */
 #define WRITE_OID_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %u", node->fldname)
 
 /* Write a long-integer field */
 #define WRITE_LONG_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %ld", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %ld", node->fldname)
 
 /* Write a char field (ie, one ascii character) */
 #define WRITE_CHAR_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outChar(str, node->fldname))
 
 /* Write an enumerated-type field as an integer code */
 #define WRITE_ENUM_FIELD(fldname, enumtype) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", \
 					 (int) node->fldname)
 
 /* Write a float field (actually, they're double) */
 #define WRITE_FLOAT_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoInternal(str, " :" CppAsString(fldname) " "), \
 	 outDouble(str, node->fldname))
 
 /* Write a boolean field */
 #define WRITE_BOOL_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %s", \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %s", \
 					 booltostr(node->fldname))
 
 /* Write a character-string (possibly NULL) field */
 #define WRITE_STRING_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outToken(str, node->fldname))
 
 /* Write a parse location field (actually same as INT case) */
 #define WRITE_LOCATION_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", write_location_fields ? node->fldname : -1)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", write_location_fields ? node->fldname : -1)
 
 /* Write a Node field */
 #define WRITE_NODE_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outNode(str, node->fldname))
 
 /* Write a bitmapset field */
 #define WRITE_BITMAPSET_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outBitmapset(str, node->fldname))
 
 /* Write a variable-length array (not a List) of Node pointers */
 #define WRITE_NODE_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeNodeArray(str, (const Node * const *) node->fldname, len))
 
 /* Write a variable-length array of AttrNumber */
 #define WRITE_ATTRNUMBER_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeAttrNumberCols(str, node->fldname, len))
 
 /* Write a variable-length array of Oid */
 #define WRITE_OID_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeOidCols(str, node->fldname, len))
 
 /* Write a variable-length array of Index */
 #define WRITE_INDEX_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeIndexCols(str, node->fldname, len))
 
 /* Write a variable-length array of int */
 #define WRITE_INT_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeIntCols(str, node->fldname, len))
 
 /* Write a variable-length array of bool */
 #define WRITE_BOOL_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeBoolCols(str, node->fldname, len))
 
 #define booltostr(x)  ((x) ? "true" : "false")
@@ -242,7 +242,7 @@ fnname(StringInfo str, const datatype *arr, int len) \
 		appendStringInfoChar(str, ')'); \
 	} \
 	else \
-		appendStringInfoString(str, "<>"); \
+		appendStringInfoStringInternal(str, "<>"); \
 }
 
 WRITE_SCALAR_ARRAY(writeAttrNumberCols, AttrNumber, " %d",)
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 2c7f778cfdb..e4abb530bf6 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -2243,7 +2243,7 @@ xml_errorHandler(void *data, PgXmlErrorPtr error)
 		void	   *errCtxSaved = xmlGenericErrorContext;
 
 		xmlSetGenericErrorFunc(&errorBuf,
-							   (xmlGenericErrorFunc) appendStringInfo);
+							   (xmlGenericErrorFunc) appendStringInfoInternal);
 
 		/* Add context information to errorBuf */
 		appendStringInfoLineSeparator(&errorBuf);
diff --git a/src/bin/psql/create_help.pl b/src/bin/psql/create_help.pl
index 12931c17b69..391aa97e2de 100644
--- a/src/bin/psql/create_help.pl
+++ b/src/bin/psql/create_help.pl
@@ -188,10 +188,15 @@ foreach (sort keys %entries)
 	$synopsis =~ s/\\n/\\n"\n$prefix"/g;
 	my @args =
 	  ("buf", $synopsis, map("_(\"$_\")", @{ $entries{$_}{params} }));
+	my $funcname = "appendPQExpBufferStr";
+	if (scalar(@args) > 2)
+	{
+		$funcname = "appendPQExpBuffer";
+	}
 	print $cfile_handle "static void
 sql_help_$id(PQExpBuffer buf)
 {
-\tappendPQExpBuffer(" . join(",\n$prefix", @args) . ");
+\t$funcname(" . join(",\n$prefix", @args) . ");
 }
 
 ";
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index ae39540e468..7a765165562 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -142,7 +142,7 @@ resetStringInfo(StringInfo str)
  * strcat.
  */
 void
-appendStringInfo(StringInfo str, const char *fmt,...)
+appendStringInfoInternal(StringInfo str, const char *fmt, ...)
 {
 	int			save_errno = errno;
 
@@ -221,13 +221,13 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
 }
 
 /*
- * appendStringInfoString
+ * appendStringInfoStringInternal
  *
  * Append a null-terminated string to str.
  * Like appendStringInfo(str, "%s", s) but faster.
  */
 void
-appendStringInfoString(StringInfo str, const char *s)
+appendStringInfoStringInternal(StringInfo str, const char *s)
 {
 	appendBinaryStringInfo(str, s, strlen(s));
 }
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index 38fffbd036b..5688ffd408c 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -1056,7 +1056,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
 	int			dcnt;
 
 #define WHEREAND() \
-	(appendPQExpBufferStr(buf, have_where ? "  AND " : "WHERE "), \
+	(appendPQExpBufferStrInternal(buf, have_where ? "  AND " : "WHERE "), \
 	 have_where = true, added_clause = true)
 
 	if (dotcnt == NULL)
@@ -1068,7 +1068,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
 		if (visibilityrule)
 		{
 			WHEREAND();
-			appendPQExpBuffer(buf, "%s\n", visibilityrule);
+			appendPQExpBufferInternal(buf, "%s\n", visibilityrule);
 		}
 		return added_clause;
 	}
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 079652c8ce4..7e5df622727 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -196,9 +196,20 @@ extern void resetStringInfo(StringInfo str);
  * to str if necessary.  This is sort of like a combination of sprintf and
  * strcat.
  */
-extern void appendStringInfo(StringInfo str, const char *fmt,...) pg_attribute_printf(2, 3);
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendStringInfo(str, fmt, ...) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(fmt) || strcmp(fmt, "%s") != 0,   \
+						 "use appendStringInfoString instead of appendStringInfo with %s");   \
+		appendStringInfoInternal(str, fmt, __VA_ARGS__); \
+	} while (0)
+#else
+#define appendStringInfo(str, fmt, ...) appendStringInfoInternal(str, fmt, __VA_ARGS__)
+#endif
 
-/*------------------------
+extern void appendStringInfoInternal(StringInfo str, const char *fmt,...) pg_attribute_printf(2, 3);
+
+ /*------------------------
  * appendStringInfoVA
  * Attempt to format text data under the control of fmt (an sprintf-style
  * format string) and append it to whatever is already in str.  If successful
@@ -214,7 +225,18 @@ extern int	appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_
  * Append a null-terminated string to str.
  * Like appendStringInfo(str, "%s", s) but faster.
  */
-extern void appendStringInfoString(StringInfo str, const char *s);
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendStringInfoString(str, s) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(s) || strlen(s) != 1, \
+						 "use appendStringInfoChar to append single characters to a string"); \
+		appendStringInfoStringInternal(str, s); \
+	 } while (0)
+#else
+#define appendStringInfoString(str, s) appendStringInfoStringInternal(str, s)
+#endif
+
+extern void appendStringInfoStringInternal(StringInfo str, const char *s);
 
 /*------------------------
  * appendStringInfoChar
diff --git a/src/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c
index abbef93f95f..ea9518a4288 100644
--- a/src/interfaces/libpq-oauth/oauth-curl.c
+++ b/src/interfaces/libpq-oauth/oauth-curl.c
@@ -417,13 +417,13 @@ append_actx_error(PGoauthBearerRequestV2 *req, struct async_ctx *actx)
  */
 
 #define actx_error(ACTX, FMT, ...) \
-	appendPQExpBuffer(&(ACTX)->errbuf, libpq_gettext(FMT), ##__VA_ARGS__)
+	appendPQExpBufferInternal(&(ACTX)->errbuf, libpq_gettext(FMT), ##__VA_ARGS__)
 
 #define actx_error_internal(ACTX, FMT, ...) \
 	appendPQExpBuffer(&(ACTX)->errbuf, FMT, ##__VA_ARGS__)
 
 #define actx_error_str(ACTX, S) \
-	appendPQExpBufferStr(&(ACTX)->errbuf, S)
+	appendPQExpBufferStrInternal(&(ACTX)->errbuf, S)
 
 /*
  * Macros for getting and setting state for the connection's two libcurl
@@ -507,7 +507,7 @@ struct oauth_parse
 };
 
 #define oauth_parse_set_error(ctx, fmt, ...) \
-	appendPQExpBuffer((ctx)->errbuf, libpq_gettext(fmt), ##__VA_ARGS__)
+	appendPQExpBufferInternal((ctx)->errbuf, libpq_gettext(fmt), ##__VA_ARGS__)
 
 #define oauth_parse_set_error_internal(ctx, fmt, ...) \
 	appendPQExpBuffer((ctx)->errbuf, fmt, ##__VA_ARGS__)
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 1e3d5bd5867..0bcfe857914 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -74,7 +74,7 @@ PQoidValue                71
 PQclientEncoding          72
 PQenv2encoding            73
 appendBinaryPQExpBuffer   74
-appendPQExpBufferStr      75
+appendPQExpBufferStrInternal      75
 destroyPQExpBuffer        76
 createPQExpBuffer         77
 PQconninfoFree            78
@@ -90,7 +90,7 @@ PQfreeNotify              87
 PQescapeString            88
 PQescapeBytea             89
 printfPQExpBuffer         90
-appendPQExpBuffer         91
+appendPQExpBufferInternal 91
 pg_encoding_to_char       92
 pg_utf_mblen              93
 PQunescapeBytea           94
diff --git a/src/interfaces/libpq/pqexpbuffer.c b/src/interfaces/libpq/pqexpbuffer.c
index 153ae6af6cb..a7730118127 100644
--- a/src/interfaces/libpq/pqexpbuffer.c
+++ b/src/interfaces/libpq/pqexpbuffer.c
@@ -254,7 +254,7 @@ printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
 }
 
 /*
- * appendPQExpBuffer
+ * appendPQExpBufferInternal
  *
  * Format text data under the control of fmt (an sprintf-like format string)
  * and append it to whatever is already in str.  More space is allocated
@@ -262,7 +262,7 @@ printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
  * strcat.
  */
 void
-appendPQExpBuffer(PQExpBuffer str, const char *fmt,...)
+appendPQExpBufferInternal(PQExpBuffer str, const char *fmt,...)
 {
 	int			save_errno = errno;
 	va_list		args;
@@ -364,7 +364,7 @@ appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args)
  * if necessary.
  */
 void
-appendPQExpBufferStr(PQExpBuffer str, const char *data)
+appendPQExpBufferStrInternal(PQExpBuffer str, const char *data)
 {
 	appendBinaryPQExpBuffer(str, data, strlen(data));
 }
diff --git a/src/interfaces/libpq/pqexpbuffer.h b/src/interfaces/libpq/pqexpbuffer.h
index dc83f4ffe86..57df4104b8f 100644
--- a/src/interfaces/libpq/pqexpbuffer.h
+++ b/src/interfaces/libpq/pqexpbuffer.h
@@ -155,7 +155,18 @@ extern void printfPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute
  * to str if necessary.  This is sort of like a combination of sprintf and
  * strcat.
  */
-extern void appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute_printf(2, 3);
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendPQExpBuffer(str, fmt, ...) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(fmt) || strcmp(fmt, "%s") != 0,   \
+						 "use appendPQExpBufferStr instead of appendPQExpBuffer with %s");   \
+		appendPQExpBufferInternal(str, fmt, __VA_ARGS__); \
+	} while (0)
+#else
+#define appendPQExpBuffer(str, fmt, ...) appendPQExpBufferInternal(str, fmt, __VA_ARGS__)
+#endif
+
+extern void appendPQExpBufferInternal(PQExpBuffer str, const char *fmt,...) pg_attribute_printf(2, 3);
 
 /*------------------------
  * appendPQExpBufferVA
@@ -167,12 +178,23 @@ extern void appendPQExpBuffer(PQExpBuffer str, const char *fmt,...) pg_attribute
  */
 extern bool appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args) pg_attribute_printf(2, 0);
 
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendPQExpBufferStr(str, s) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(s) || strlen(s) != 1, \
+						 "use appendPQExpBufferChar to append single characters to a string"); \
+		appendPQExpBufferStrInternal(str, s); \
+	 } while (0)
+#else
+#define appendPQExpBufferStr(str, s) appendPQExpBufferStrInternal(str, s)
+#endif
+
 /*------------------------
  * appendPQExpBufferStr
  * Append the given string to a PQExpBuffer, allocating more space
  * if necessary.
  */
-extern void appendPQExpBufferStr(PQExpBuffer str, const char *data);
+extern void appendPQExpBufferStrInternal(PQExpBuffer str, const char *data);
 
 /*------------------------
  * appendPQExpBufferChar
-- 
2.51.0