Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests
Bruce Momjian <[email protected]> Wed, 16 Mar 2005 01:00:21 -0500 (EST)
| Newsgroups | gmane.comp.db.postgresql.devel.patches,gmane.comp.db.postgresql.devel.general,gmane.comp.db.postgresql.devel.win32 |
|---|---|
| Message-ID | <[email protected]> |
I have applied a modified version of your patch, attached. initdb will not work without %*s support, so I had to add that. Please look over my work. I don't think i handle %*1$ but I am not even sure what that means or if our translators would ever use such a thing. You can probably test that better than I can. Your patch didn't handle signed vs. unsigned va_arg values properly.. There were also maxwidth tests around 's' that I had to remove to support %*. I think those tests are down in fmtstr too but please check. initdb and regression pass. --------------------------------------------------------------------------- Nicolai Tufar wrote: > Resubmission of yesterday's patch so that it would > cont conflict with Bruce's cvs commit. Pleas apply. > > Best regards, > Nicolai. > > > On Sat, 12 Mar 2005 01:58:15 +0200, Nicolai Tufar <[email protected]> wrote: > > On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian > > <[email protected]> wrote: > > > > The CVS-tip implementation is fundamentally broken and won't work even > > > > for our internal uses. I've not wasted time complaining about it > > > > because I thought we were going to replace it. If we can't find a > > > > usable replacement then we're going to have to put a lot of effort > > > > into fixing what's there. On the whole I think the effort would be > > > > better spent importing someone else's solution. > > > > > > Oh, so our existing implementation doesn't even meet our needs. OK. > > > > Which made me wander why did I not aggree with > > Tom Lane's suggestion to make do three passes > > instead of two. Tom was right, as usual. It happened to > > be much easier than I expected. The patch is attached. > > Please apply. > > > > Tom, what do you think? Will it be fine with you? > > > > Best regards, > > Nicolai > > > > > > [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us [email protected] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 ---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster
/bjm/diff
(text/plain, 14.3 KB)
Index: src/port/snprintf.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/snprintf.c,v
retrieving revision 1.19
diff -c -c -r1.19 snprintf.c
*** src/port/snprintf.c 12 Mar 2005 04:00:56 -0000 1.19
--- src/port/snprintf.c 16 Mar 2005 05:46:33 -0000
***************
*** 151,170 ****
#define FMTSTR 1
#define FMTNUM 2
! #define FMTFLOAT 3
! #define FMTCHAR 4
static void
dopr(char *buffer, const char *format, va_list args, char *end)
{
int ch;
- int64 value;
- double fvalue;
int longlongflag = 0;
int longflag = 0;
int pointflag = 0;
int maxwidth = 0;
- char *strvalue;
int ljust;
int len;
int zpad;
--- 151,170 ----
#define FMTSTR 1
#define FMTNUM 2
! #define FMTNUM_U 3
! #define FMTFLOAT 4
! #define FMTCHAR 5
! #define FMTWIDTH 6
! #define FMTLEN 7
static void
dopr(char *buffer, const char *format, va_list args, char *end)
{
int ch;
int longlongflag = 0;
int longflag = 0;
int pointflag = 0;
int maxwidth = 0;
int ljust;
int len;
int zpad;
***************
*** 173,178 ****
--- 173,179 ----
const char* fmtbegin;
int fmtpos = 1;
int realpos = 0;
+ int precision;
int position;
char *output;
int percents = 1;
***************
*** 195,200 ****
--- 196,203 ----
int pointflag;
char func;
int realpos;
+ int longflag;
+ int longlongflag;
} *fmtpar, **fmtparptr;
/* Create enough structures to hold all arguments */
***************
*** 229,240 ****
longflag = longlongflag = pointflag = 0;
fmtbegin = format - 1;
realpos = 0;
! position = 0;
nextch:
ch = *format++;
switch (ch)
{
! case 0:
goto performpr;
case '-':
ljust = 1;
--- 232,243 ----
longflag = longlongflag = pointflag = 0;
fmtbegin = format - 1;
realpos = 0;
! position = precision = 0;
nextch:
ch = *format++;
switch (ch)
{
! case '\0':
goto performpr;
case '-':
ljust = 1;
***************
*** 251,274 ****
case '7':
case '8':
case '9':
! if (pointflag)
! /* could also be precision */
! maxwidth = maxwidth * 10 + ch - '0';
! else
{
len = len * 10 + ch - '0';
position = position * 10 + ch - '0';
}
goto nextch;
case '$':
realpos = position;
len = 0;
goto nextch;
case '*':
! if (pointflag)
! maxwidth = va_arg(args, int);
else
! len = va_arg(args, int);
goto nextch;
case '.':
pointflag = 1;
--- 254,282 ----
case '7':
case '8':
case '9':
! if (!pointflag)
{
len = len * 10 + ch - '0';
position = position * 10 + ch - '0';
}
+ else
+ {
+ maxwidth = maxwidth * 10 + ch - '0';
+ precision = precision * 10 + ch - '0';
+ }
goto nextch;
case '$':
realpos = position;
len = 0;
goto nextch;
case '*':
! MemSet(&fmtpar[fmtpos], 0, sizeof(fmtpar[fmtpos]));
! if (!pointflag)
! fmtpar[fmtpos].func = FMTLEN;
else
! fmtpar[fmtpos].func = FMTWIDTH;
! fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
! fmtpos++;
goto nextch;
case '.':
pointflag = 1;
***************
*** 301,368 ****
#endif
case 'u':
case 'U':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].numvalue = value;
fmtpar[fmtpos].base = 10;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'o':
case 'O':
! /* fmtnum(value,base,dosign,ljust,len,zpad,&output) */
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].numvalue = value;
fmtpar[fmtpos].base = 8;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'd':
case 'D':
! if (longflag)
! {
! if (longlongflag)
! {
! value = va_arg(args, int64);
! }
! else
! value = va_arg(args, long);
! }
! else
! value = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].numvalue = value;
fmtpar[fmtpos].base = 10;
fmtpar[fmtpos].dosign = 1;
fmtpar[fmtpos].ljust = ljust;
--- 309,348 ----
#endif
case 'u':
case 'U':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].base = 10;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM_U;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'o':
case 'O':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].base = 8;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM_U;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'd':
case 'D':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].base = 10;
fmtpar[fmtpos].dosign = 1;
fmtpar[fmtpos].ljust = ljust;
***************
*** 373,444 ****
fmtpos++;
break;
case 'x':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].numvalue = value;
fmtpar[fmtpos].base = 16;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'X':
! if (longflag)
! {
! if (longlongflag)
! value = va_arg(args, uint64);
! else
! value = va_arg(args, unsigned long);
! }
! else
! value = va_arg(args, unsigned int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].numvalue = value;
fmtpar[fmtpos].base = -16;
fmtpar[fmtpos].dosign = 1;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 's':
! strvalue = va_arg(args, char *);
! if (maxwidth > 0 || !pointflag)
! {
! if (pointflag && len > maxwidth)
! len = maxwidth; /* Adjust padding */
! fmtpar[fmtpos].fmtbegin = fmtbegin;
! fmtpar[fmtpos].fmtend = format;
! fmtpar[fmtpos].value = strvalue;
! fmtpar[fmtpos].ljust = ljust;
! fmtpar[fmtpos].len = len;
! fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].maxwidth = maxwidth;
! fmtpar[fmtpos].func = FMTSTR;
! fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
! fmtpos++;
! }
break;
case 'c':
- ch = va_arg(args, int);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].charvalue = ch;
fmtpar[fmtpos].func = FMTCHAR;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
--- 353,399 ----
fmtpos++;
break;
case 'x':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].base = 16;
fmtpar[fmtpos].dosign = 0;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM_U;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 'X':
! fmtpar[fmtpos].longflag = longflag;
! fmtpar[fmtpos].longlongflag = longlongflag;
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].base = -16;
fmtpar[fmtpos].dosign = 1;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].func = FMTNUM_U;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
break;
case 's':
! fmtpar[fmtpos].fmtbegin = fmtbegin;
! fmtpar[fmtpos].fmtend = format;
! fmtpar[fmtpos].ljust = ljust;
! fmtpar[fmtpos].len = len;
! fmtpar[fmtpos].zpad = zpad;
! fmtpar[fmtpos].maxwidth = maxwidth;
! fmtpar[fmtpos].func = FMTSTR;
! fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
! fmtpos++;
break;
case 'c':
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].func = FMTCHAR;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
fmtpos++;
***************
*** 448,462 ****
case 'f':
case 'g':
case 'G':
- fvalue = va_arg(args, double);
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
- fmtpar[fmtpos].fvalue = fvalue;
fmtpar[fmtpos].type = ch;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].maxwidth = maxwidth;
! fmtpar[fmtpos].precision = position;
fmtpar[fmtpos].pointflag = pointflag;
fmtpar[fmtpos].func = FMTFLOAT;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
--- 403,415 ----
case 'f':
case 'g':
case 'G':
fmtpar[fmtpos].fmtbegin = fmtbegin;
fmtpar[fmtpos].fmtend = format;
fmtpar[fmtpos].type = ch;
fmtpar[fmtpos].ljust = ljust;
fmtpar[fmtpos].len = len;
fmtpar[fmtpos].maxwidth = maxwidth;
! fmtpar[fmtpos].precision = precision;
fmtpar[fmtpos].pointflag = pointflag;
fmtpar[fmtpos].func = FMTFLOAT;
fmtpar[fmtpos].realpos = realpos?realpos:fmtpos;
***************
*** 473,494 ****
break;
}
}
performpr:
! /* shuffle pointers */
for(i = 1; i < fmtpos; i++)
fmtparptr[i] = &fmtpar[fmtpar[i].realpos];
output = buffer;
format = format_save;
while ((ch = *format++))
{
for(i = 1; i < fmtpos; i++)
{
! if(ch == '%' && *format == '%')
{
format++;
continue;
}
! if(fmtpar[i].fmtbegin == format - 1)
{
switch(fmtparptr[i]->func){
case FMTSTR:
--- 426,499 ----
break;
}
}
+
performpr:
! /* reorder pointers */
for(i = 1; i < fmtpos; i++)
fmtparptr[i] = &fmtpar[fmtpar[i].realpos];
+
+ /* assign values */
+ for(i = 1; i < fmtpos; i++){
+ switch(fmtparptr[i]->func){
+ case FMTSTR:
+ fmtparptr[i]->value = va_arg(args, char *);
+ break;
+ case FMTNUM:
+ if (fmtparptr[i]->longflag)
+ {
+ if (fmtparptr[i]->longlongflag)
+ fmtparptr[i]->numvalue = va_arg(args, int64);
+ else
+ fmtparptr[i]->numvalue = va_arg(args, long);
+ }
+ else
+ fmtparptr[i]->numvalue = va_arg(args, int);
+ break;
+ case FMTNUM_U:
+ if (fmtparptr[i]->longflag)
+ {
+ if (fmtparptr[i]->longlongflag)
+ fmtparptr[i]->numvalue = va_arg(args, uint64);
+ else
+ fmtparptr[i]->numvalue = va_arg(args, unsigned long);
+ }
+ else
+ fmtparptr[i]->numvalue = va_arg(args, unsigned int);
+ break;
+ case FMTFLOAT:
+ fmtparptr[i]->fvalue = va_arg(args, double);
+ break;
+ case FMTCHAR:
+ fmtparptr[i]->charvalue = va_arg(args, int);
+ break;
+ case FMTLEN:
+ if (i + 1 < fmtpos && fmtpar[i + 1].func != FMTWIDTH)
+ fmtpar[i + 1].len = va_arg(args, int);
+ /* For "%*.*f", use the second arg */
+ if (i + 2 < fmtpos && fmtpar[i + 1].func == FMTWIDTH)
+ fmtpar[i + 2].len = va_arg(args, int);
+ break;
+ case FMTWIDTH:
+ if (i + 1 < fmtpos)
+ fmtpar[i + 1].maxwidth = fmtpar[i + 1].precision =
+ va_arg(args, int);
+ break;
+ }
+ }
+
+ /* do the output */
output = buffer;
format = format_save;
while ((ch = *format++))
{
for(i = 1; i < fmtpos; i++)
{
! if (ch == '%' && *format == '%')
{
format++;
continue;
}
! if (fmtpar[i].fmtbegin == format - 1)
{
switch(fmtparptr[i]->func){
case FMTSTR:
***************
*** 497,502 ****
--- 502,508 ----
fmtparptr[i]->maxwidth, end, &output);
break;
case FMTNUM:
+ case FMTNUM_U:
fmtnum(fmtparptr[i]->numvalue, fmtparptr[i]->base,
fmtparptr[i]->dosign, fmtparptr[i]->ljust,
fmtparptr[i]->len, fmtparptr[i]->zpad, end, &output);