Thread: Re: [HACKERS] \x output blowing up

Re: [HACKERS] \x output blowing up

From
Bruce Momjian
Date:
Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Sat, Sep 24, 2005 at 07:18:16PM -0400, Bruce Momjian wrote:
> >
> > Well, it seems we are going to have to fix it somehow for 8.1.  It is
> > not crashing here so I can't work up a patch.  Can you submit a minimal
> > fix for 8.1?  Thanks.
>
> Ah, it would only happen if your encoding was UTF-8 since that's the
> only case psql handles differently. I've attached a patch which fixes
> it. With a bit more rearrangement you could probably simplify it a bit
> but this works.

Fixed. You were right that the use of cell_w was incorrect for
non-numeric values (UTF8), and in fact was just too fragile to use.

I redesigned format_numeric_locale() to return an allocated result,
which removed this problem and simplified the code too.

Patch attached and applied.

--
  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/bin/psql/print.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.75
diff -c -c -r1.75 print.c
*** src/bin/psql/print.c    26 Sep 2005 18:09:57 -0000    1.75
--- src/bin/psql/print.c    27 Sep 2005 16:25:54 -0000
***************
*** 86,107 ****
      return strlen(my_str) + additional_numeric_locale_len(my_str);
  }

! static void
! format_numeric_locale(char *my_str)
  {
      int i, j, int_len = integer_digits(my_str), leading_digits;
      int    groupdigits = atoi(grouping);
!     char *new_str;
!
!     if (my_str[0] == '-')
!         my_str++;
!
!     new_str = pg_local_malloc(strlen_with_numeric_locale(my_str) + 1);

      leading_digits = (int_len % groupdigits != 0) ?
                       int_len % groupdigits : groupdigits;

!     for (i=0, j=0; ; i++, j++)
      {
          /* Hit decimal point? */
          if (my_str[i] == '.')
--- 86,111 ----
      return strlen(my_str) + additional_numeric_locale_len(my_str);
  }

! static char *
! format_numeric_locale(const char *my_str)
  {
      int i, j, int_len = integer_digits(my_str), leading_digits;
      int    groupdigits = atoi(grouping);
!     int    new_str_start = 0;
!     char *new_str = new_str = pg_local_malloc(
!                               strlen_with_numeric_locale(my_str) + 1);

      leading_digits = (int_len % groupdigits != 0) ?
                       int_len % groupdigits : groupdigits;

!     if (my_str[0] == '-')    /* skip over sign, affects grouping calculations */
!     {
!         new_str[0] = my_str[0];
!         my_str++;
!         new_str_start = 1;
!     }
!
!     for (i=0, j=new_str_start; ; i++, j++)
      {
          /* Hit decimal point? */
          if (my_str[i] == '.')
***************
*** 130,137 ****
          new_str[j] = my_str[i];
      }

!     strcpy(my_str, new_str);
!     free(new_str);
  }

  /*************************/
--- 134,140 ----
          new_str[j] = my_str[i];
      }

!     return new_str;
  }

  /*************************/
***************
*** 185,194 ****
          }
          if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              fputs(my_cell, fout);
              free(my_cell);
          }
--- 188,195 ----
          }
          if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              fputs(my_cell, fout);
              free(my_cell);
          }
***************
*** 261,270 ****
          fputs(opt_fieldsep, fout);
          if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              fputs(my_cell, fout);
              free(my_cell);
          }
--- 262,269 ----
          fputs(opt_fieldsep, fout);
          if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              fputs(my_cell, fout);
              free(my_cell);
          }
***************
*** 488,500 ****
          {
              if (opt_numeric_locale)
              {
!                 char *my_cell = pg_local_malloc(cell_w[i] + 1);

-                 strcpy(my_cell, *ptr);
-                 format_numeric_locale(my_cell);
                  fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", my_cell);
                  free(my_cell);
!             }
              else
                  fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", *ptr);
          }
--- 487,497 ----
          {
              if (opt_numeric_locale)
              {
!                 char *my_cell = format_numeric_locale(*ptr);

                  fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", my_cell);
                  free(my_cell);
!             }
              else
                  fprintf(fout, "%*s%s", widths[i % col_count] - cell_w[i], "", *ptr);
          }
***************
*** 697,714 ****
          else
              fputs(" ", fout);

          {
!             char *my_cell = pg_local_malloc(cell_w[i] + 1);
!
!             strcpy(my_cell, *ptr);
!             if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
!                 format_numeric_locale(my_cell);
              if (opt_border < 2)
                  fprintf(fout, "%s\n", my_cell);
              else
                  fprintf(fout, "%-s%*s |\n", my_cell, dwidth - cell_w[i], "");
              free(my_cell);
          }
      }

      if (opt_border == 2)
--- 694,716 ----
          else
              fputs(" ", fout);

+         if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              if (opt_border < 2)
                  fprintf(fout, "%s\n", my_cell);
              else
                  fprintf(fout, "%-s%*s |\n", my_cell, dwidth - cell_w[i], "");
              free(my_cell);
          }
+         else
+         {
+             if (opt_border < 2)
+                 fprintf(fout, "%s\n", *ptr);
+             else
+                 fprintf(fout, "%-s%*s |\n", *ptr, dwidth - cell_w[i], "");
+         }
      }

      if (opt_border == 2)
***************
*** 837,846 ****
              fputs("  ", fout);
          else if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);

-             strcpy(my_cell, *ptr);
-             format_numeric_locale(my_cell);
              html_escaped_print(my_cell, fout);
              free(my_cell);
          }
--- 839,846 ----
              fputs("  ", fout);
          else if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);

              html_escaped_print(my_cell, fout);
              free(my_cell);
          }
***************
*** 922,931 ****
              fputs("  ", fout);
          else if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              html_escaped_print(my_cell, fout);
              free(my_cell);
          }
--- 922,929 ----
              fputs("  ", fout);
          else if (opt_align[i % col_count] == 'r' && opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              html_escaped_print(my_cell, fout);
              free(my_cell);
          }
***************
*** 1064,1073 ****
      {
          if (opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              latex_escaped_print(my_cell, fout);
              free(my_cell);
          }
--- 1062,1069 ----
      {
          if (opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              latex_escaped_print(my_cell, fout);
              free(my_cell);
          }
***************
*** 1177,1186 ****
          {
              if (opt_numeric_locale)
              {
!                 char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);

-                 strcpy(my_cell, *ptr);
-                 format_numeric_locale(my_cell);
                  latex_escaped_print(my_cell, fout);
                  free(my_cell);
              }
--- 1173,1180 ----
          {
              if (opt_numeric_locale)
              {
!                 char *my_cell = format_numeric_locale(*ptr);

                  latex_escaped_print(my_cell, fout);
                  free(my_cell);
              }
***************
*** 1277,1286 ****
      {
          if (opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              troff_ms_escaped_print(my_cell, fout);
              free(my_cell);
          }
--- 1271,1278 ----
      {
          if (opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              troff_ms_escaped_print(my_cell, fout);
              free(my_cell);
          }
***************
*** 1389,1398 ****
          fputc('\t', fout);
          if (opt_numeric_locale)
          {
!             char *my_cell = pg_local_malloc(strlen_with_numeric_locale(*ptr) + 1);
!
!             strcpy(my_cell, *ptr);
!             format_numeric_locale(my_cell);
              troff_ms_escaped_print(my_cell, fout);
              free(my_cell);
          }
--- 1381,1388 ----
          fputc('\t', fout);
          if (opt_numeric_locale)
          {
!             char *my_cell = format_numeric_locale(*ptr);
!
              troff_ms_escaped_print(my_cell, fout);
              free(my_cell);
          }

Re: [HACKERS] \x output blowing up

From
Martijn van Oosterhout
Date:
On Tue, Sep 27, 2005 at 12:32:14PM -0400, Bruce Momjian wrote:
> I redesigned format_numeric_locale() to return an allocated result,
> which removed this problem and simplified the code too.

Much better, I had that in my own tree but figured it wasn't "minimal"
enough for this stage of beta. It is, IMHO, the much better solution.

Thanks for the fix.
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Attachment