diff --git a/config/c-library.m4 b/config/c-library.m4 index 4446a5b..6bcfcee 100644 *** a/config/c-library.m4 --- b/config/c-library.m4 *************** int main() *** 238,243 **** --- 238,270 ---- AC_MSG_RESULT([$pgac_cv_snprintf_size_t_support]) ])# PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT + # PGAC_FUNC_SNPRINTF_C99_RESULT + # ----------------------------- + # Determine whether snprintf returns the desired buffer length when + # it overruns the actual buffer length. That's required by C99 and POSIX + # but ancient platforms don't behave that way, so we must test. + # + AC_DEFUN([PGAC_FUNC_SNPRINTF_C99_RESULT], + [AC_MSG_CHECKING([whether snprintf reports buffer overrun per C99]) + AC_CACHE_VAL(pgac_cv_snprintf_c99_result, + [AC_RUN_IFELSE([AC_LANG_SOURCE([[#include + #include + + int main() + { + char buf[10]; + + if (snprintf(buf, sizeof(buf), "12345678901234567890") != 20) + return 1; + return 0; + }]])], + [pgac_cv_snprintf_c99_result=yes], + [pgac_cv_snprintf_c99_result=no], + [pgac_cv_snprintf_c99_result=cross]) + ])dnl AC_CACHE_VAL + AC_MSG_RESULT([$pgac_cv_snprintf_c99_result]) + ])# PGAC_FUNC_SNPRINTF_C99_RESULT + # PGAC_TYPE_LOCALE_T # ------------------ diff --git a/configure b/configure index 067fc43..863d07f 100755 *** a/configure --- b/configure *************** fi *** 15965,15971 **** # Run tests below here # -------------------- ! # Force use of our snprintf if system's doesn't do arg control # See comment above at snprintf test for details. if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf supports argument control" >&5 --- 15965,15971 ---- # Run tests below here # -------------------- ! # For NLS, force use of our snprintf if system's doesn't do arg control. # See comment above at snprintf test for details. if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf supports argument control" >&5 *************** $as_echo "$pgac_cv_snprintf_size_t_suppo *** 16259,16264 **** --- 16259,16308 ---- fi fi + # Force use of our snprintf if the system's doesn't handle buffer overrun + # as specified by C99. + if test "$pgac_need_repl_snprintf" = no; then + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether snprintf reports buffer overrun per C99" >&5 + $as_echo_n "checking whether snprintf reports buffer overrun per C99... " >&6; } + if ${pgac_cv_snprintf_c99_result+:} false; then : + $as_echo_n "(cached) " >&6 + else + if test "$cross_compiling" = yes; then : + pgac_cv_snprintf_c99_result=cross + else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext + /* end confdefs.h. */ + #include + #include + + int main() + { + char buf[10]; + + if (snprintf(buf, sizeof(buf), "12345678901234567890") != 20) + return 1; + return 0; + } + _ACEOF + if ac_fn_c_try_run "$LINENO"; then : + pgac_cv_snprintf_c99_result=yes + else + pgac_cv_snprintf_c99_result=no + fi + rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \ + conftest.$ac_objext conftest.beam conftest.$ac_ext + fi + + + fi + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_snprintf_c99_result" >&5 + $as_echo "$pgac_cv_snprintf_c99_result" >&6; } + + if test "$pgac_cv_snprintf_c99_result" != yes; then + pgac_need_repl_snprintf=yes + fi + fi + # Now we have checked all the reasons to replace snprintf if test $pgac_need_repl_snprintf = yes; then diff --git a/configure.in b/configure.in index 49257e5..1c529e7 100644 *** a/configure.in --- b/configure.in *************** for the exact reason.]])], *** 1805,1811 **** # Run tests below here # -------------------- ! # Force use of our snprintf if system's doesn't do arg control # See comment above at snprintf test for details. if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then PGAC_FUNC_SNPRINTF_ARG_CONTROL --- 1805,1811 ---- # Run tests below here # -------------------- ! # For NLS, force use of our snprintf if system's doesn't do arg control. # See comment above at snprintf test for details. if test "$enable_nls" = yes -a "$pgac_need_repl_snprintf" = no; then PGAC_FUNC_SNPRINTF_ARG_CONTROL *************** if test "$pgac_need_repl_snprintf" = no; *** 1857,1862 **** --- 1857,1871 ---- fi fi + # Force use of our snprintf if the system's doesn't handle buffer overrun + # as specified by C99. + if test "$pgac_need_repl_snprintf" = no; then + PGAC_FUNC_SNPRINTF_C99_RESULT + if test "$pgac_cv_snprintf_c99_result" != yes; then + pgac_need_repl_snprintf=yes + fi + fi + # Now we have checked all the reasons to replace snprintf if test $pgac_need_repl_snprintf = yes; then AC_DEFINE(USE_REPL_SNPRINTF, 1, [Use replacement snprintf() functions.]) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index f458c0e..9989d3a 100644 *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** do_serialize(char **destptr, Size *maxby *** 9441,9466 **** if (*maxbytes <= 0) elog(ERROR, "not enough space to serialize GUC state"); - errno = 0; - va_start(vargs, fmt); n = vsnprintf(*destptr, *maxbytes, fmt, vargs); va_end(vargs); ! /* ! * Cater to portability hazards in the vsnprintf() return value just like ! * appendPQExpBufferVA() does. Note that this requires an extra byte of ! * slack at the end of the buffer. Since serialize_variable() ends with a ! * do_serialize_binary() rather than a do_serialize(), we'll always have ! * that slack; estimate_variable_size() need not add a byte for it. ! */ ! if (n < 0 || n >= *maxbytes - 1) { ! if (n < 0 && errno != 0 && errno != ENOMEM) ! /* Shouldn't happen. Better show errno description. */ ! elog(ERROR, "vsnprintf failed: %m"); ! else ! elog(ERROR, "not enough space to serialize GUC state"); } /* Shift the destptr ahead of the null terminator */ --- 9441,9459 ---- if (*maxbytes <= 0) elog(ERROR, "not enough space to serialize GUC state"); va_start(vargs, fmt); n = vsnprintf(*destptr, *maxbytes, fmt, vargs); va_end(vargs); ! if (n < 0) { ! /* Shouldn't happen. Better show errno description. */ ! elog(ERROR, "vsnprintf failed: %m"); ! } ! if (n >= *maxbytes) ! { ! /* This shouldn't happen either, really. */ ! elog(ERROR, "not enough space to serialize GUC state"); } /* Shift the destptr ahead of the null terminator */ diff --git a/src/common/psprintf.c b/src/common/psprintf.c index b974a99..04465a1 100644 *** a/src/common/psprintf.c --- b/src/common/psprintf.c *************** psprintf(const char *fmt,...) *** 77,83 **** * 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, len > 0). * * If successful, return the number of bytes emitted, not counting the * trailing zero byte. This will always be strictly less than len. --- 77,83 ---- * 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). * * If successful, return the number of bytes emitted, not counting the * trailing zero byte. This will always be strictly less than len. *************** psprintf(const char *fmt,...) *** 89,102 **** * 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. */ --- 89,99 ---- * Other error cases do not return, but exit via elog(ERROR) or exit(). * Hence, this shouldn't be used inside libpq. * * 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. + * (Given a C99-compliant vsnprintf, that won't happen, but it is rumored + * that some implementations don't always return the same value ...) * Second, we return the recommended buffer size, not one less than that; * this lets overflow concerns be handled here rather than in the callers. */ *************** pvsnprintf(char *buf, size_t len, const *** 105,132 **** { int nprinted; - Assert(len > 0); - - errno = 0; - - /* - * 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 - buf[len - 1] = '\0'; - #endif - nprinted = vsnprintf(buf, len, fmt, args); ! Assert(buf[len - 1] == '\0'); ! ! /* ! * If vsnprintf reports an error other than ENOMEM, fail. The possible ! * causes of this are not user-facing errors, so elog should be enough. ! */ ! if (nprinted < 0 && errno != 0 && errno != ENOMEM) { #ifndef FRONTEND elog(ERROR, "vsnprintf failed: %m"); --- 102,111 ---- { int nprinted; nprinted = vsnprintf(buf, len, fmt, args); ! /* We assume failure means the fmt is bogus, hence hard failure is OK */ ! if (unlikely(nprinted < 0)) { #ifndef FRONTEND elog(ERROR, "vsnprintf failed: %m"); *************** pvsnprintf(char *buf, size_t len, const *** 136,177 **** #endif } ! /* ! * Note: some versions of vsnprintf return the number of chars actually ! * stored, not the total space needed as C99 specifies. And at least one ! * returns -1 on failure. Be conservative about believing whether the ! * print worked. ! */ ! if (nprinted >= 0 && (size_t) nprinted < len - 1) { /* Success. Note nprinted does not include trailing null. */ 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, 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. - * - * In the corner case where the required space just barely overflows, - * fall through so that we'll error out below (possibly after - * looping). - */ - if ((size_t) nprinted <= MaxAllocSize - 2) - return nprinted + 2; - } - /* ! * Buffer overrun, and we don't know how much space is needed. Estimate ! * 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) { #ifndef FRONTEND ereport(ERROR, --- 115,135 ---- #endif } ! if ((size_t) nprinted < len) { /* Success. Note nprinted does not include trailing null. */ return (size_t) nprinted; } /* ! * We assume a C99-compliant vsnprintf, so believe its estimate of the ! * required space, and add one for the trailing null. (If it's wrong, the ! * logic will still work, but we may loop multiple times.) ! * ! * Choke if the required space would exceed MaxAllocSize. Note we use ! * this palloc-oriented overflow limit even when in frontend. */ ! if (unlikely((size_t) nprinted > MaxAllocSize - 1)) { #ifndef FRONTEND ereport(ERROR, *************** pvsnprintf(char *buf, size_t len, const *** 183,190 **** #endif } ! if (len >= MaxAllocSize / 2) ! return MaxAllocSize; ! ! return len * 2; } --- 141,145 ---- #endif } ! return nprinted + 1; } diff --git a/src/interfaces/libpq/pqexpbuffer.c b/src/interfaces/libpq/pqexpbuffer.c index 86b16e6..0814ec6 100644 *** a/src/interfaces/libpq/pqexpbuffer.c --- b/src/interfaces/libpq/pqexpbuffer.c *************** appendPQExpBufferVA(PQExpBuffer str, con *** 295,370 **** */ if (str->maxlen > str->len + 16) { ! /* ! * Note: we intentionally leave one byte unused, as a guard against ! * old broken versions of vsnprintf. ! */ ! avail = str->maxlen - str->len - 1; ! ! errno = 0; nprinted = vsnprintf(str->data + str->len, avail, fmt, args); /* ! * If vsnprintf reports an error other than ENOMEM, fail. */ ! if (nprinted < 0 && errno != 0 && errno != ENOMEM) { markPQExpBufferBroken(str); return true; } ! /* ! * Note: some versions of vsnprintf return the number of chars ! * actually stored, not the total space needed as C99 specifies. And ! * at least one returns -1 on failure. Be conservative about ! * believing whether the print worked. ! */ ! if (nprinted >= 0 && (size_t) nprinted < avail - 1) { /* Success. Note nprinted does not include trailing null. */ str->len += nprinted; return true; } ! if (nprinted >= 0 && (size_t) nprinted > avail) ! { ! /* ! * This appears to be a C99-compliant vsnprintf, so believe its ! * 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. ! * ! * In the corner case where the required space just barely ! * overflows, fail. ! */ ! if (nprinted > INT_MAX - 2) ! { ! markPQExpBufferBroken(str); ! return true; ! } ! needed = nprinted + 2; ! } ! else { ! /* ! * Buffer overrun, and we don't know how much space is needed. ! * Estimate twice the previous buffer size, but not more than ! * INT_MAX. ! */ ! if (avail >= INT_MAX / 2) ! needed = INT_MAX; ! else ! needed = avail * 2; } } else { /* * We have to guess at how much to enlarge, since we're skipping the ! * formatting work. */ needed = 32; } --- 295,344 ---- */ if (str->maxlen > str->len + 16) { ! avail = str->maxlen - str->len; nprinted = vsnprintf(str->data + str->len, avail, fmt, args); /* ! * If vsnprintf reports an error, fail (we assume this means there's ! * something wrong with the format string). */ ! if (unlikely(nprinted < 0)) { markPQExpBufferBroken(str); return true; } ! if ((size_t) nprinted < avail) { /* Success. Note nprinted does not include trailing null. */ str->len += nprinted; return true; } ! /* ! * We assume a C99-compliant vsnprintf, so believe its estimate of the ! * required space, and add one for the trailing null. (If it's wrong, ! * the logic will still work, but we may loop multiple times.) ! * ! * Choke if the required space would exceed INT_MAX, since str->maxlen ! * can't represent more than that. ! */ ! if (unlikely(nprinted > INT_MAX - 1)) { ! markPQExpBufferBroken(str); ! return true; } + needed = nprinted + 1; } else { /* * We have to guess at how much to enlarge, since we're skipping the ! * formatting work. Fortunately, because of enlargePQExpBuffer's ! * preference for power-of-2 sizes, this number isn't very sensitive; ! * the net effect is that we'll double the buffer size before trying ! * to run vsnprintf, which seems sensible. */ needed = 32; }