Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests
Date
Msg-id 200503160600.j2G60LX09825@candle.pha.pa.us
Whole thread Raw
In response to Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail  (Nicolai Tufar <ntufar@gmail.com>)
Responses Re: [pgsql-hackers-win32] [HACKERS] snprintf causes regression tests to fail  (Nicolai Tufar <ntufar@gmail.com>)
List pgsql-patches
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 <ntufar@gmail.com> wrote:
> > On Thu, 10 Mar 2005 19:21:41 -0500 (EST), Bruce Momjian
> > <pgman@candle.pha.pa.us> 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
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
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);

pgsql-patches by date:

Previous
From: Neil Conway
Date:
Subject: Re: Sort psql output
Next
From: Bruce Momjian
Date:
Subject: Re: [COMMITTERS] pgsql: Handle carriage returns and line feeds in COPY