dbTalk Databases Forums  

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

mailing.database.pgsql-bugs mailing.database.pgsql-bugs


Discuss [BUGS] BUG #1044: snprintf() shipped with PostgreSQL is not thread safe in the mailing.database.pgsql-bugs forum.



Reply
 
Thread Tools Display Modes
  #1  
Old   
PostgreSQL Bugs List
 
Posts: n/a

Default [BUGS] BUG #1044: snprintf() shipped with PostgreSQL is not thread safe - 01-08-2004 , 01:57 AM







The following bug has been logged online:

Bug reference: 1044
Logged by: Denis Stepanov

Email address: D.N.Stepanov (AT) inp (DOT) 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++;
}



---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

Reply With Quote
  #2  
Old   
Tom Lane
 
Posts: n/a

Default Re: [BUGS] BUG #1044: snprintf() shipped with PostgreSQL is not thread safe - 01-08-2004 , 08:55 AM






"PostgreSQL Bugs List" <pgsql-bugs (AT) postgresql (DOT) org> writes:
Quote:
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.
What platforms have workable thread support but not snprintf? I think
this change is not likely to accomplish much except clutter the snprintf
code ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match


Reply With Quote
  #3  
Old   
Tom Lane
 
Posts: n/a

Default Re: [BUGS] BUG #1044: snprintf() shipped with PostgreSQL is not thread safe - 01-08-2004 , 11:25 AM



Bruce Momjian <pgman (AT) candle (DOT) pha.pa.us> writes:
Quote:
What we could do is to throw a compile #error if you hit our own
snprintf() and are compiling with threads enabled.
Good thought. If we get any complaints then we can reconsider applying
this patch, otherwise there's no need.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html


Reply With Quote
Reply




Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off



Powered by vBulletin Version 3.5.3
Copyright ©2000 - 2012, Jelsoft Enterprises Ltd.