From cf13f964c0e3b4f424c73f8bc6df603de69c8269 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 15 Sep 2022 21:27:00 +1200 Subject: [PATCH v1] Adjust code to highlight appendStringInfo misusages Not intended for commit --- src/backend/nodes/outfuncs.c | 40 ++++++++++++++++++------------------ src/backend/utils/adt/xml.c | 2 +- src/common/stringinfo.c | 6 +++--- src/include/lib/stringinfo.h | 30 +++++++++++++++++++++++---- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e66a99247e..a236ec9e77 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -42,92 +42,92 @@ 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 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", node->fldname) + appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", node->fldname) /* 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") @@ -233,7 +233,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 2300c7ebf3..9df1a5b72c 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -2104,7 +2104,7 @@ xml_errorHandler(void *data, xmlErrorPtr error) void *errCtxSaved = xmlGenericErrorContext; xmlSetGenericErrorFunc((void *) errorBuf, - (xmlGenericErrorFunc) appendStringInfo); + (xmlGenericErrorFunc) appendStringInfoInternal); /* Add context information to errorBuf */ appendStringInfoLineSeparator(errorBuf); diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index 05b22b5c53..a275185113 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -88,7 +88,7 @@ resetStringInfo(StringInfo str) * strcat. */ void -appendStringInfo(StringInfo str, const char *fmt,...) +appendStringInfoInternal(StringInfo str, const char *fmt, ...) { int save_errno = errno; @@ -167,13 +167,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/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 36a416f8e0..4fa1b1ea66 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -93,9 +93,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 @@ -111,7 +122,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 -- 2.40.1.windows.1