BUG #1044: snprintf() shipped with PostgreSQL is not thread safe - Mailing list pgsql-bugs

The following bug has been logged online:

Bug reference:      1044
Logged by:          Denis Stepanov

Email address:      D.N.Stepanov@inp.nsk.su

PostgreSQL version: 7.4

Operating system:   Any OS lacking proper snprintf()

Description:        snprintf() shipped with PostgreSQL is not thread safe

Details:

Some OSes lack proper snprintf()/vsnprintf() fuctions so PostgreSQL includes
its own version (src/port/snprintf.c) during building. Unfortunately, this
version of snprintf() is not reentrant (it uses global vars to keep internal
state), so for example running libpq-based concurrent applications (threads)
causes libpq fuctions to fail sometimes. The suggestion is to remove globals
usage in src/port/snprintf.c. I am attaching here a sample patch for your
review. Thanks.

--- postgresql-7.4.1/src/port/snprintf.c    Thu Jul 18 11:13:59 2002
+++ postgresql-7.4.1.fixed/src/port/snprintf.c    Thu Jan  8 13:23:47 2004
@@ -67,6 +67,9 @@
  * Sigh.  This sort of thing is always nasty do deal with.    Note that
  * the version here does not include floating point. (now it does ... tgl)
  *
+ * Denis Stepanov Thu Jan  8 13:01:01 NOVT 2004
+ * Made functions reentrant (thread safe).
+ *
  * snprintf() is used instead of sprintf() as it does limit checks
  * for string length.  This covers a nasty loophole.
  *
@@ -75,12 +78,18 @@
  **************************************************************/

 /*static char _id[] = "$Id: snprintf.c,v 1.1 2002/07/18 04:13:59 momjian
Exp $";*/
-static char *end;
-static int    SnprfOverflow;
+
+typedef struct _vsnprintf_data
+{
+    int SnprfOverflow;
+    char *end;
+    char *output;
+
+} vsnprintf_data;

 int            snprintf(char *str, size_t count, const char *fmt,...);
 int            vsnprintf(char *str, size_t count, const char *fmt, va_list args);
-static void dopr(char *buffer, const char *format, va_list args);
+static void dopr(char *buffer, const char *format, va_list args,
vsnprintf_data *vsnpd);

 int
 snprintf(char *str, size_t count, const char *fmt,...)
@@ -98,12 +107,14 @@
 int
 vsnprintf(char *str, size_t count, const char *fmt, va_list args)
 {
+    vsnprintf_data vsnpd;
+
     str[0] = '\0';
-    end = str + count - 1;
-    SnprfOverflow = 0;
-    dopr(str, fmt, args);
+    vsnpd.end = str + count - 1;
+    vsnpd.SnprfOverflow = 0;
+    dopr(str, fmt, args, &vsnpd);
     if (count > 0)
-        end[0] = '\0';
+        vsnpd.end[0] = '\0';
     return strlen(str);
 }

@@ -111,17 +122,16 @@
  * dopr(): poor man's version of doprintf
  */

-static void fmtstr(char *value, int ljust, int len, int zpad, int
maxwidth);
-static void fmtnum(long_long value, int base, int dosign, int ljust, int
len, int zpad);
-static void fmtfloat(double value, char type, int ljust, int len, int
precision, int pointflag);
-static void dostr(char *str, int cut);
-static void dopr_outch(int c);
+static void fmtstr(char *value, int ljust, int len, int zpad, int maxwidth,
vsnprintf_data *vsnpd);
+static void fmtnum(long_long value, int base, int dosign, int ljust, int
len, int zpad, vsnprintf_data *vsnpd);
+static void fmtfloat(double value, char type, int ljust, int len, int
precision, int pointflag, vsnprintf_data *vsnpd);
+static void dostr(char *str, int cut, vsnprintf_data *vsnpd);
+static void dopr_outch(int c, vsnprintf_data *vsnpd);

-static char *output;


 static void
-dopr(char *buffer, const char *format, va_list args)
+dopr(char *buffer, const char *format, va_list args, vsnprintf_data *vsnpd)
 {
     int            ch;
     long_long    value;
@@ -135,7 +145,7 @@
     int            len;
     int            zpad;

-    output = buffer;
+    vsnpd->output = buffer;
     while ((ch = *format++))
     {
         switch (ch)
@@ -148,8 +158,8 @@
                 switch (ch)
                 {
                     case 0:
-                        dostr("**end of format**", 0);
-                        *output = '\0';
+                        dostr("**end of format**", 0, vsnpd);
+                        *vsnpd->output = '\0';
                         return;
                     case '-':
                         ljust = 1;
@@ -188,7 +198,7 @@
                         goto nextch;
                     case 'u':
                     case 'U':
-                        /* fmtnum(value,base,dosign,ljust,len,zpad) */
+                        /* fmtnum(value,base,dosign,ljust,len,zpad,vsnpd) */
                         if (longflag)
                         {
                             if (longlongflag)
@@ -198,11 +208,11 @@
                         }
                         else
                             value = va_arg(args, unsigned int);
-                        fmtnum(value, 10, 0, ljust, len, zpad);
+                        fmtnum(value, 10, 0, ljust, len, zpad, vsnpd);
                         break;
                     case 'o':
                     case 'O':
-                        /* fmtnum(value,base,dosign,ljust,len,zpad) */
+                        /* fmtnum(value,base,dosign,ljust,len,zpad,vsnpd) */
                         if (longflag)
                         {
                             if (longlongflag)
@@ -212,7 +222,7 @@
                         }
                         else
                             value = va_arg(args, unsigned int);
-                        fmtnum(value, 8, 0, ljust, len, zpad);
+                        fmtnum(value, 8, 0, ljust, len, zpad, vsnpd);
                         break;
                     case 'd':
                     case 'D':
@@ -225,7 +235,7 @@
                         }
                         else
                             value = va_arg(args, int);
-                        fmtnum(value, 10, 1, ljust, len, zpad);
+                        fmtnum(value, 10, 1, ljust, len, zpad, vsnpd);
                         break;
                     case 'x':
                         if (longflag)
@@ -237,7 +247,7 @@
                         }
                         else
                             value = va_arg(args, unsigned int);
-                        fmtnum(value, 16, 0, ljust, len, zpad);
+                        fmtnum(value, 16, 0, ljust, len, zpad, vsnpd);
                         break;
                     case 'X':
                         if (longflag)
@@ -249,7 +259,7 @@
                         }
                         else
                             value = va_arg(args, unsigned int);
-                        fmtnum(value, -16, 0, ljust, len, zpad);
+                        fmtnum(value, -16, 0, ljust, len, zpad, vsnpd);
                         break;
                     case 's':
                         strvalue = va_arg(args, char *);
@@ -257,12 +267,12 @@
                         {
                             if (pointflag && len > maxwidth)
                                 len = maxwidth; /* Adjust padding */
-                            fmtstr(strvalue, ljust, len, zpad, maxwidth);
+                            fmtstr(strvalue, ljust, len, zpad, maxwidth, vsnpd);
                         }
                         break;
                     case 'c':
                         ch = va_arg(args, int);
-                        dopr_outch(ch);
+                        dopr_outch(ch, vsnpd);
                         break;
                     case 'e':
                     case 'E':
@@ -270,25 +280,25 @@
                     case 'g':
                     case 'G':
                         fvalue = va_arg(args, double);
-                        fmtfloat(fvalue, ch, ljust, len, maxwidth, pointflag);
+                        fmtfloat(fvalue, ch, ljust, len, maxwidth, pointflag, vsnpd);
                         break;
                     case '%':
-                        dopr_outch(ch);
+                        dopr_outch(ch, vsnpd);
                         continue;
                     default:
-                        dostr("???????", 0);
+                        dostr("???????", 0, vsnpd);
                 }
                 break;
             default:
-                dopr_outch(ch);
+                dopr_outch(ch, vsnpd);
                 break;
         }
     }
-    *output = '\0';
+    *vsnpd->output = '\0';
 }

 static void
-fmtstr(char *value, int ljust, int len, int zpad, int maxwidth)
+fmtstr(char *value, int ljust, int len, int zpad, int maxwidth,
vsnprintf_data *vsnpd)
 {
     int            padlen,
                 strlen;            /* amount to pad */
@@ -305,19 +315,19 @@
         padlen = -padlen;
     while (padlen > 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         --padlen;
     }
-    dostr(value, maxwidth);
+    dostr(value, maxwidth, vsnpd);
     while (padlen < 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         ++padlen;
     }
 }

 static void
-fmtnum(long_long value, int base, int dosign, int ljust, int len, int zpad)
+fmtnum(long_long value, int base, int dosign, int ljust, int len, int zpad,
vsnprintf_data *vsnpd)
 {
     int            signvalue = 0;
     ulong_long    uvalue;
@@ -372,34 +382,34 @@
     {
         if (signvalue)
         {
-            dopr_outch(signvalue);
+            dopr_outch(signvalue, vsnpd);
             --padlen;
             signvalue = 0;
         }
         while (padlen > 0)
         {
-            dopr_outch(zpad);
+            dopr_outch(zpad, vsnpd);
             --padlen;
         }
     }
     while (padlen > 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         --padlen;
     }
     if (signvalue)
-        dopr_outch(signvalue);
+        dopr_outch(signvalue, vsnpd);
     while (place > 0)
-        dopr_outch(convert[--place]);
+        dopr_outch(convert[--place], vsnpd);
     while (padlen < 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         ++padlen;
     }
 }

 static void
-fmtfloat(double value, char type, int ljust, int len, int precision, int
pointflag)
+fmtfloat(double value, char type, int ljust, int len, int precision, int
pointflag, vsnprintf_data *vsnpd)
 {
     char        fmt[32];
     char        convert[512];
@@ -426,45 +436,45 @@

     while (padlen > 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         --padlen;
     }
-    dostr(convert, 0);
+    dostr(convert, 0, vsnpd);
     while (padlen < 0)
     {
-        dopr_outch(' ');
+        dopr_outch(' ', vsnpd);
         ++padlen;
     }
 }

 static void
-dostr(char *str, int cut)
+dostr(char *str, int cut, vsnprintf_data *vsnpd)
 {
     if (cut)
     {
         while (*str && cut-- > 0)
-            dopr_outch(*str++);
+            dopr_outch(*str++, vsnpd);
     }
     else
     {
         while (*str)
-            dopr_outch(*str++);
+            dopr_outch(*str++, vsnpd);
     }
 }

 static void
-dopr_outch(int c)
+dopr_outch(int c, vsnprintf_data *vsnpd)
 {
 #ifdef NOT_USED
     if (iscntrl((unsigned char) c) && c != '\n' && c != '\t')
     {
         c = '@' + (c & 0x1F);
-        if (end == 0 || output < end)
-            *output++ = '^';
+        if (vsnpd->end == 0 || vsnpd->output < vsnpd->end)
+            *vsnpd->output++ = '^';
     }
 #endif
-    if (end == 0 || output < end)
-        *output++ = c;
+    if (vsnpd->end == 0 || vsnpd->output < vsnpd->end)
+        *vsnpd->output++ = c;
     else
-        SnprfOverflow++;
+        vsnpd->SnprfOverflow++;
 }