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

1 files changed, 36 insertions, 23 deletions

diff --git a/src/common/psprintf.c b/src/common/psprintf.c
index 788c8f0d69..41c39c4c32 100644
--- a/src/common/psprintf.c
+++ b/src/common/psprintf.c
@@ -23,10 +23,6 @@
 #include "utils/memutils.h"
 
 
-static size_t pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
-__attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0)));
-
-
 /*
  * psprintf
  *
@@ -48,6 +44,7 @@ psprintf(const char *fmt,...)
 	{
 		char	   *result;
 		va_list		args;
+		size_t		newlen;
 
 		/*
 		 * Allocate result buffer.	Note that in frontend this maps to malloc
@@ -57,14 +54,15 @@ psprintf(const char *fmt,...)
 
 		/* Try to format the data. */
 		va_start(args, fmt);
-		len = pvsnprintf(result, len, fmt, args);
+		newlen = pvsnprintf(result, len, fmt, args);
 		va_end(args);
 
-		if (len == 0)
+		if (newlen < len)
 			return result;		/* success */
 
 		/* Release buffer and loop around to try again with larger len. */
 		pfree(result);
+		len = newlen;
 	}
 }
 
@@ -72,19 +70,30 @@ psprintf(const char *fmt,...)
  * pvsnprintf
  *
  * Attempt to format text data under the control of fmt (an sprintf-style
- * format string) and insert it into buf (which has length len).
+ * format string) and insert it into buf (which has length len, len > 0).
+ *
+ * If successful, return the number of bytes emitted, not counting the
+ * trailing zero byte.  This will always be strictly less than len.
  *
- * If successful, return zero.	If there's not enough space in buf, return
- * an estimate of the buffer size needed to succeed (this *must* be more
- * than "len", else psprintf might loop infinitely).
- * Other error cases do not return.
+ * If there's not enough space in buf, return an estimate of the buffer size
+ * needed to succeed (this *must* be more than the given len, else callers
+ * might loop infinitely).
  *
- * 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.
+ * Other error cases do not return, but exit via elog(ERROR) or exit().
+ * Hence, this shouldn't be used inside libpq.
+ *
+ * This function exists mainly to centralize our workarounds for
+ * non-C99-compliant vsnprintf implementations.  Generally, any call that
+ * pays any attention to the return value should go through here rather
+ * than calling snprintf or vsnprintf directly.
+ *
+ * Note that the semantics of the return value are not exactly C99's.
+ * First, we don't promise that the estimated buffer size is exactly right;
+ * callers must be prepared to loop multiple times to get the right size.
+ * Second, we return the recommended buffer size, not one less than that;
+ * this lets overflow concerns be handled here rather than in the callers.
  */
-static size_t
+size_t
 pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
 {
 	int			nprinted;
@@ -129,15 +138,15 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
 	if (nprinted >= 0 && (size_t) nprinted < len - 1)
 	{
 		/* Success.  Note nprinted does not include trailing null. */
-		return 0;
+		return (size_t) nprinted;
 	}
 
 	if (nprinted >= 0 && (size_t) nprinted > len)
 	{
 		/*
 		 * This appears to be a C99-compliant vsnprintf, so believe its
-		 * estimate of the required space.	(If it's wrong, this code will
-		 * still work, but may loop multiple times.)  Note that the space
+		 * estimate of the required space.	(If it's wrong, the logic will
+		 * still work, but we may loop multiple times.)  Note that the space
 		 * needed should be only nprinted+1 bytes, but we'd better allocate
 		 * one more than that so that the test above will succeed next time.
 		 *
@@ -150,14 +159,15 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
 
 	/*
 	 * Buffer overrun, and we don't know how much space is needed.  Estimate
-	 * twice the previous buffer size.	If this would overflow, choke.	We use
-	 * a palloc-oriented overflow limit even when in frontend.
+	 * twice the previous buffer size, but not more than MaxAllocSize; if we
+	 * are already at MaxAllocSize, choke.  Note we use this palloc-oriented
+	 * overflow limit even when in frontend.
 	 */
-	if (len > MaxAllocSize / 2)
+	if (len >= MaxAllocSize)
 	{
 #ifndef FRONTEND
 		ereport(ERROR,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
+				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 				 errmsg("out of memory")));
 #else
 		fprintf(stderr, _("out of memory\n"));
@@ -165,5 +175,8 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
 #endif
 	}
 
+	if (len >= MaxAllocSize / 2)
+		return MaxAllocSize;
+
 	return len * 2;
 }