Use improved vsnprintf calling logic in more places.

When we are using a C99-compliant vsnprintf implementation (which should be
most places, these days) it is worth the trouble to make use of its report
of how large the buffer needs to be to succeed.  This patch adjusts
stringinfo.c and some miscellaneous usages in pg_dump to do that, relying
on the logic recently added in libpgcommon's psprintf.c.  Since these
places want to know the number of bytes written once we succeed, modify the
API of pvsnprintf() to report that.

There remains near-duplicate logic in pqexpbuffer.c, but since that code
is in libpq, psprintf.c's approach of exit()-on-error isn't appropriate
for use there.  Also note that I didn't bother touching the multitude
of places that call (v)snprintf without any attempt to provide a resizable
buffer.

Release-note-worthy incompatibility: the API of appendStringInfoVA()
changed.  If there's any third-party code that's calling that directly,
it will need tweaking along the same lines as in this patch.

David Rowley and Tom Lane

Author: tglsfdc

src/backend/lib/stringinfo.c

@@ -80,18 +80,18 @@ appendStringInfo(StringInfo str, const char *fmt,...)
 	for (;;)
 	{
 		va_list		args;
-		bool		success;
+		int			needed;
 
 		/* Try to format the data. */
 		va_start(args, fmt);
-		success = appendStringInfoVA(str, fmt, args);
+		needed = appendStringInfoVA(str, fmt, args);
 		va_end(args);
 
-		if (success)
-			break;
+		if (needed == 0)
+			break;				/* success */
 
-		/* Double the buffer size and try again. */
-		enlargeStringInfo(str, str->maxlen);
+		/* Increase the buffer size and try again. */
+		enlargeStringInfo(str, needed);
 	}
 }
 
@@ -100,59 +100,51 @@ appendStringInfo(StringInfo str, const char *fmt,...)
  *
  * 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
- * return true; if not (because there's not enough space), return false
- * without modifying str.  Typically the caller would enlarge str and retry
- * on false return --- see appendStringInfo for standard usage pattern.
+ * return zero; if not (because there's not enough space), return an estimate
+ * of the space needed, without modifying str.  Typically the caller should
+ * pass the return value to enlargeStringInfo() before trying again; see
+ * appendStringInfo for standard usage pattern.
  *
  * XXX This API is ugly, but there seems no alternative given the C spec's
  * restrictions on what can portably be done with va_list arguments: you have
  * to redo va_start before you can rescan the argument list, and we can't do
  * that from here.
  */
-bool
+int
 appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
 {
-	int			avail,
-				nprinted;
+	int			avail;
+	size_t		nprinted;
 
 	Assert(str != NULL);
 
 	/*
 	 * If there's hardly any space, don't bother trying, just fail to make the
-	 * caller enlarge the buffer first.
+	 * caller enlarge the buffer first.  We have to guess at how much to
+	 * enlarge, since we're skipping the formatting work.
 	 */
-	avail = str->maxlen - str->len - 1;
+	avail = str->maxlen - str->len;
 	if (avail < 16)
-		return false;
+		return 32;
 
-	/*
-	 * Assert check here is to catch buggy vsnprintf that overruns the
-	 * specified buffer length.  Solaris 7 in 64-bit mode is an example of a
-	 * platform with such a bug.
-	 */
-#ifdef USE_ASSERT_CHECKING
-	str->data[str->maxlen - 1] = '\0';
-#endif
-
-	nprinted = vsnprintf(str->data + str->len, avail, fmt, args);
+	nprinted = pvsnprintf(str->data + str->len, (size_t) avail, fmt, args);
 
-	Assert(str->data[str->maxlen - 1] == '\0');
-
-	/*
-	 * Note: some versions of vsnprintf return the number of chars actually
-	 * stored, but at least one returns -1 on failure. Be conservative about
-	 * believing whether the print worked.
-	 */
-	if (nprinted >= 0 && nprinted < avail - 1)
+	if (nprinted < (size_t) avail)
 	{
 		/* Success.  Note nprinted does not include trailing null. */
-		str->len += nprinted;
-		return true;
+		str->len += (int) nprinted;
+		return 0;
 	}
 
 	/* Restore the trailing null so that str is unmodified. */
 	str->data[str->len] = '\0';
-	return false;
+
+	/*
+	 * Return pvsnprintf's estimate of the space needed.  (Although this is
+	 * given as a size_t, we know it will fit in int because it's not more
+	 * than MaxAllocSize.)
+	 */
+	return (int) nprinted;
 }
 
 /*

src/backend/utils/error/elog.c

@@ -715,13 +715,13 @@ errcode_for_socket_access(void)
 		for (;;) \
 		{ \
 			va_list		args; \
-			bool		success; \
+			int			needed; \
 			va_start(args, fmt); \
-			success = appendStringInfoVA(&buf, fmtbuf, args); \
+			needed = appendStringInfoVA(&buf, fmtbuf, args); \
 			va_end(args); \
-			if (success) \
+			if (needed == 0) \
 				break; \
-			enlargeStringInfo(&buf, buf.maxlen); \
+			enlargeStringInfo(&buf, needed); \
 		} \
 		/* Done with expanded fmt */ \
 		pfree(fmtbuf); \
@@ -758,13 +758,13 @@ errcode_for_socket_access(void)
 		for (;;) \
 		{ \
 			va_list		args; \
-			bool		success; \
+			int			needed; \
 			va_start(args, n); \
-			success = appendStringInfoVA(&buf, fmtbuf, args); \
+			needed = appendStringInfoVA(&buf, fmtbuf, args); \
 			va_end(args); \
-			if (success) \
+			if (needed == 0) \
 				break; \
-			enlargeStringInfo(&buf, buf.maxlen); \
+			enlargeStringInfo(&buf, needed); \
 		} \
 		/* Done with expanded fmt */ \
 		pfree(fmtbuf); \

src/bin/pg_dump/pg_backup_archiver.c

@@ -1183,29 +1183,33 @@ archputs(const char *s, Archive *AH)
 int
 archprintf(Archive *AH, const char *fmt,...)
 {
-	char	   *p = NULL;
-	va_list		ap;
-	int			bSize = strlen(fmt) + 256;
-	int			cnt = -1;
+	char	   *p;
+	size_t		len = 128;		/* initial assumption about buffer size */
+	size_t		cnt;
 
-	/*
-	 * This is paranoid: deal with the possibility that vsnprintf is willing
-	 * to ignore trailing null or returns > 0 even if string does not fit. It
-	 * may be the case that it returns cnt = bufsize
-	 */
-	while (cnt < 0 || cnt >= (bSize - 1))
+	for (;;)
 	{
-		if (p != NULL)
-			free(p);
-		bSize *= 2;
-		p = (char *) pg_malloc(bSize);
-		va_start(ap, fmt);
-		cnt = vsnprintf(p, bSize, fmt, ap);
-		va_end(ap);
+		va_list		args;
+
+		/* Allocate work buffer. */
+		p = (char *) pg_malloc(len);
+
+		/* Try to format the data. */
+		va_start(args, fmt);
+		cnt = pvsnprintf(p, len, fmt, args);
+		va_end(args);
+
+		if (cnt < len)
+			break;				/* success */
+
+		/* Release buffer and loop around to try again with larger len. */
+		free(p);
+		len = cnt;
 	}
+
 	WriteData(AH, p, cnt);
 	free(p);
-	return cnt;
+	return (int) cnt;
 }
 
 
@@ -1312,29 +1316,33 @@ RestoreOutput(ArchiveHandle *AH, OutputContext savedContext)
 int
 ahprintf(ArchiveHandle *AH, const char *fmt,...)
 {
-	char	   *p = NULL;
-	va_list		ap;
-	int			bSize = strlen(fmt) + 256;		/* Usually enough */
-	int			cnt = -1;
+	char	   *p;
+	size_t		len = 128;		/* initial assumption about buffer size */
+	size_t		cnt;
 
-	/*
-	 * This is paranoid: deal with the possibility that vsnprintf is willing
-	 * to ignore trailing null or returns > 0 even if string does not fit. It
-	 * may be the case that it returns cnt = bufsize.
-	 */
-	while (cnt < 0 || cnt >= (bSize - 1))
+	for (;;)
 	{
-		if (p != NULL)
-			free(p);
-		bSize *= 2;
-		p = (char *) pg_malloc(bSize);
-		va_start(ap, fmt);
-		cnt = vsnprintf(p, bSize, fmt, ap);
-		va_end(ap);
+		va_list		args;
+
+		/* Allocate work buffer. */
+		p = (char *) pg_malloc(len);
+
+		/* Try to format the data. */
+		va_start(args, fmt);
+		cnt = pvsnprintf(p, len, fmt, args);
+		va_end(args);
+
+		if (cnt < len)
+			break;				/* success */
+
+		/* Release buffer and loop around to try again with larger len. */
+		free(p);
+		len = cnt;
 	}
+
 	ahwrite(p, 1, cnt, AH);
 	free(p);
-	return cnt;
+	return (int) cnt;
 }
 
 void

src/bin/pg_dump/pg_backup_tar.c

@@ -996,33 +996,33 @@ _EndBlobs(ArchiveHandle *AH, TocEntry *te)
 static int
 tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt,...)
 {
-	char	   *p = NULL;
-	va_list		ap;
-	size_t		bSize = strlen(fmt) + 256;		/* Should be enough */
-	int			cnt = -1;
-
-	/*
-	 * This is paranoid: deal with the possibility that vsnprintf is willing
-	 * to ignore trailing null
-	 */
+	char	   *p;
+	size_t		len = 128;		/* initial assumption about buffer size */
+	size_t		cnt;
 
-	/*
-	 * or returns > 0 even if string does not fit. It may be the case that it
-	 * returns cnt = bufsize
-	 */
-	while (cnt < 0 || cnt >= (bSize - 1))
+	for (;;)
 	{
-		if (p != NULL)
-			free(p);
-		bSize *= 2;
-		p = (char *) pg_malloc(bSize);
-		va_start(ap, fmt);
-		cnt = vsnprintf(p, bSize, fmt, ap);
-		va_end(ap);
+		va_list		args;
+
+		/* Allocate work buffer. */
+		p = (char *) pg_malloc(len);
+
+		/* Try to format the data. */
+		va_start(args, fmt);
+		cnt = pvsnprintf(p, len, fmt, args);
+		va_end(args);
+
+		if (cnt < len)
+			break;				/* success */
+
+		/* Release buffer and loop around to try again with larger len. */
+		free(p);
+		len = cnt;
 	}
+
 	cnt = tarWrite(p, cnt, th);
 	free(p);
-	return cnt;
+	return (int) cnt;
 }
 
 bool