]> granicus.if.org Git - postgresql/commitdiff
Set snprintf.c's maximum number of NL arguments to be 31.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Oct 2018 16:41:28 +0000 (12:41 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Oct 2018 16:41:28 +0000 (12:41 -0400)
Previously, we used the platform's NL_ARGMAX if any, otherwise 16.
The trouble with this is that the platform value is hugely variable,
ranging from the POSIX-minimum 9 to as much as 64K on recent FreeBSD.
Values of more than a dozen or two have no practical use and slow down
the initialization of the argtypes array.  Worse, they cause snprintf.c
to consume far more stack space than was the design intention, possibly
resulting in stack-overflow crashes.

Standardize on 31, which is comfortably more than we need (it looks like
no existing translatable message has more than about 10 parameters).
I chose that, not 32, to make the array sizes powers of 2, for some
possible small gain in speed of the memset.

The lack of reported crashes suggests that the set of platforms we
use snprintf.c on (in released branches) may have no overlap with
the set where NL_ARGMAX has unreasonably large values.  But that's
not entirely clear, so back-patch to all supported branches.

Per report from Mateusz Guzik (via Thomas Munro).

Discussion: https://postgr.es/m/CAEepm=3VF=PUp2f8gU8fgZB22yPE_KBS0+e1AHAtQ=09schTHg@mail.gmail.com

src/port/snprintf.c

index 2c77eec1c6ba388c2b0bbf87b12805167f47f500..1be5f70bda55e6f08e28230cd1ad214165a5bda2 100644 (file)
 #endif
 #include <sys/param.h>
 
-#ifndef NL_ARGMAX
-#define NL_ARGMAX 16
-#endif
+/*
+ * We used to use the platform's NL_ARGMAX here, but that's a bad idea,
+ * first because the point of this module is to remove platform dependencies
+ * not perpetuate them, and second because some platforms use ridiculously
+ * large values, leading to excessive stack consumption in dopr().
+ */
+#define PG_NL_ARGMAX 31
 
 
 /*
@@ -358,8 +362,8 @@ dopr(PrintfTarget *target, const char *format, va_list args)
        double          fvalue;
        char       *strvalue;
        int                     i;
-       PrintfArgType argtypes[NL_ARGMAX + 1];
-       PrintfArgValue argvalues[NL_ARGMAX + 1];
+       PrintfArgType argtypes[PG_NL_ARGMAX + 1];
+       PrintfArgValue argvalues[PG_NL_ARGMAX + 1];
 
        /*
         * Parse the format string to determine whether there are %n$ format
@@ -409,7 +413,7 @@ nextch1:
                                goto nextch1;
                        case '$':
                                have_dollar = true;
-                               if (accum <= 0 || accum > NL_ARGMAX)
+                               if (accum <= 0 || accum > PG_NL_ARGMAX)
                                        goto bad_format;
                                if (afterstar)
                                {