From d615eb695d426f72f94dc57c421bbc1aa10ade83 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 | 28 ++++++++++++++++++++++--- 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 3337b77ae6..f65f08b0e2 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -45,92 +45,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", 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") @@ -236,7 +236,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 3e4ca874d8..09c515501d 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -2135,7 +2135,7 @@ xml_errorHandler(void *data, PgXmlErrorPtr 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 ec5fc2422d..a6c2939a73 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -94,7 +94,7 @@ resetStringInfo(StringInfo str) * strcat. */ void -appendStringInfo(StringInfo str, const char *fmt,...) +appendStringInfoInternal(StringInfo str, const char *fmt, ...) { int save_errno = errno; @@ -173,13 +173,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 cd9632e3fc..882614416c 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -169,9 +169,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 @@ -187,7 +198,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