BUG #1044: snprintf() shipped with PostgreSQL is not thread safe

The following bug has been logged online:

Bug reference: 1044
Logged by: Denis Stepanov

Email address: D(dot)N(dot)Stepanov(at)inp(dot)nsk(dot)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++;
}