diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c index a5b9f2e03e..016eac4891 100644 --- a/src/backend/lib/stringinfo.c +++ b/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 = pvsnprintf(str->data + str->len, (size_t) avail, fmt, args); - nprinted = vsnprintf(str->data + str->len, 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; } /* diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 9c7489a9bb..a2c8680fd0 100644 --- a/src/backend/utils/error/elog.c +++ b/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); \ diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 50619a2815..1e189fe8ad 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/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 diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c index 2358b9d160..06005ae868 100644 --- a/src/bin/pg_dump/pg_backup_tar.c +++ b/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; + 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; } + cnt = tarWrite(p, cnt, th); free(p); - return cnt; + return (int) cnt; } bool 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 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 successful, return the number of bytes emitted, not counting the + * trailing zero byte. This will always be strictly less than len. * - * 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. + * 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). + * + * 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; } diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index ecb693fcc2..cece81208e 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -101,11 +101,12 @@ __attribute__((format(PG_PRINTF_ATTRIBUTE, 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 - * 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. */ -extern bool +extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0))); diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h index 676333fd6d..e327c81c69 100644 --- a/src/include/utils/palloc.h +++ b/src/include/utils/palloc.h @@ -98,14 +98,16 @@ extern char *MemoryContextStrdup(MemoryContext context, const char *string); extern char *pstrdup(const char *in); extern char *pnstrdup(const char *in, Size len); -/* sprintf into a palloc'd buffer */ -extern char *psprintf(const char *fmt,...) -__attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); - /* basic memory allocation functions */ extern void *palloc(Size size); extern void *palloc0(Size size); extern void pfree(void *pointer); extern void *repalloc(void *pointer, Size size); +/* sprintf into a palloc'd buffer --- these are in psprintf.c */ +extern char *psprintf(const char *fmt,...) +__attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); +extern size_t pvsnprintf(char *buf, size_t len, const char *fmt, va_list args) +__attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0))); + #endif /* PALLOC_H */ diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c index 44d35a747b..77cd4273ba 100644 --- a/src/pl/plpython/plpy_elog.c +++ b/src/pl/plpython/plpy_elog.c @@ -70,14 +70,14 @@ PLy_elog(int elevel, const char *fmt,...) for (;;) { va_list ap; - bool success; + int needed; va_start(ap, fmt); - success = appendStringInfoVA(&emsg, dgettext(TEXTDOMAIN, fmt), ap); + needed = appendStringInfoVA(&emsg, dgettext(TEXTDOMAIN, fmt), ap); va_end(ap); - if (success) + if (needed == 0) break; - enlargeStringInfo(&emsg, emsg.maxlen); + enlargeStringInfo(&emsg, needed); } primary = emsg.data;