Fix assorted bogosities in cash_in() and cash_out().
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 29 Oct 2011 18:31:15 +0000 (14:31 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 29 Oct 2011 18:31:15 +0000 (14:31 -0400)
cash_out failed to handle multiple-byte thousands separators, as per bug
#6277 from Alexander Law.  In addition, cash_in didn't handle that either,
nor could it handle multiple-byte positive_sign.  Both routines failed to
support multiple-byte mon_decimal_point, which I did not think was worth
changing, but at least now they check for the possibility and fall back to
using '.' rather than emitting invalid output.  Also, make cash_in handle
trailing negative signs, which formerly it would reject.  Since cash_out
generates trailing negative signs whenever the locale tells it to, this
last omission represents a fail-to-reload-dumped-data bug.  IMO that
justifies patching this all the way back.

src/backend/utils/adt/cash.c

index ca0a04536839f687dba8a07e3f816c6b0935d617..0cab38b01c2582c8f150f1c6a6fb80eea9c79534 100644 (file)
 
 static const char *num_word(Cash value);
 
-/* when we go to 64 bit values we will have to modify this */
-#define CASH_BUFSZ     24
-
-#define TERMINATOR     (CASH_BUFSZ - 1)
-#define LAST_PAREN     (TERMINATOR - 1)
-#define LAST_DIGIT     (LAST_PAREN - 1)
-
 
 /*
  * Cash is a pass-by-ref SQL type, so we must pass and return pointers.
@@ -71,14 +64,14 @@ cash_in(PG_FUNCTION_ARGS)
    Cash        value = 0;
    Cash        dec = 0;
    Cash        sgn = 1;
-   int         seen_dot = 0;
+   bool        seen_dot = false;
    const char *s = str;
    int         fpoint;
-   char       *csymbol;
-   char        dsymbol,
-               ssymbol,
-               psymbol,
-              *nsymbol;
+   char        dsymbol;
+   const char *ssymbol,
+              *psymbol,
+              *nsymbol,
+              *csymbol;
 
    struct lconv *lconvert = PGLC_localeconv();
 
@@ -96,14 +89,22 @@ cash_in(PG_FUNCTION_ARGS)
    if (fpoint < 0 || fpoint > 10)
        fpoint = 2;             /* best guess in this case, I think */
 
-   dsymbol = ((*lconvert->mon_decimal_point != '\0') ? *lconvert->mon_decimal_point : '.');
-   ssymbol = ((*lconvert->mon_thousands_sep != '\0') ? *lconvert->mon_thousands_sep : ',');
-   csymbol = ((*lconvert->currency_symbol != '\0') ? lconvert->currency_symbol : "$");
-   psymbol = ((*lconvert->positive_sign != '\0') ? *lconvert->positive_sign : '+');
-   nsymbol = ((*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-");
+   /* we restrict dsymbol to be a single byte, but not the other symbols */
+   if (*lconvert->mon_decimal_point != '\0' &&
+       lconvert->mon_decimal_point[1] == '\0')
+       dsymbol = *lconvert->mon_decimal_point;
+   else
+       dsymbol = '.';
+   if (*lconvert->mon_thousands_sep != '\0')
+       ssymbol = lconvert->mon_thousands_sep;
+   else                        /* ssymbol should not equal dsymbol */
+       ssymbol = (dsymbol != ',') ? "," : ".";
+   csymbol = (*lconvert->currency_symbol != '\0') ? lconvert->currency_symbol : "$";
+   psymbol = (*lconvert->positive_sign != '\0') ? lconvert->positive_sign : "+";
+   nsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 
 #ifdef CASHDEBUG
-   printf("cashin- precision '%d'; decimal '%c'; thousands '%c'; currency '%s'; positive '%c'; negative '%s'\n",
+   printf("cashin- precision '%d'; decimal '%c'; thousands '%s'; currency '%s'; positive '%s'; negative '%s'\n",
           fpoint, dsymbol, ssymbol, csymbol, psymbol, nsymbol);
 #endif
 
@@ -124,23 +125,20 @@ cash_in(PG_FUNCTION_ARGS)
    {
        sgn = -1;
        s += strlen(nsymbol);
-#ifdef CASHDEBUG
-       printf("cashin- negative symbol; string is '%s'\n", s);
-#endif
    }
    else if (*s == '(')
    {
        sgn = -1;
        s++;
-
    }
-   else if (*s == psymbol)
-       s++;
+   else if (strncmp(s, psymbol, strlen(psymbol)) == 0)
+       s += strlen(psymbol);
 
 #ifdef CASHDEBUG
    printf("cashin- string is '%s'\n", s);
 #endif
 
+   /* allow whitespace and currency symbol after the sign, too */
    while (isspace((unsigned char) *s))
        s++;
    if (strncmp(s, csymbol, strlen(csymbol)) == 0)
@@ -150,7 +148,7 @@ cash_in(PG_FUNCTION_ARGS)
    printf("cashin- string is '%s'\n", s);
 #endif
 
-   for (;; s++)
+   for (; *s; s++)
    {
        /* we look for digits as long as we have found less */
        /* than the required number of decimal places */
@@ -164,30 +162,44 @@ cash_in(PG_FUNCTION_ARGS)
        /* decimal point? then start counting fractions... */
        else if (*s == dsymbol && !seen_dot)
        {
-           seen_dot = 1;
+           seen_dot = true;
        }
        /* ignore if "thousands" separator, else we're done */
-       else if (*s != ssymbol)
-       {
-           /* round off */
-           if (isdigit((unsigned char) *s) && *s >= '5')
-               value++;
-
-           /* adjust for less than required decimal places */
-           for (; dec < fpoint; dec++)
-               value *= 10;
-
+       else if (strncmp(s, ssymbol, strlen(ssymbol)) == 0)
+           s += strlen(ssymbol) - 1;
+       else
            break;
-       }
    }
 
-   while (isspace((unsigned char) *s) || *s == '0' || *s == ')')
-       s++;
+   /* round off if there's another digit */
+   if (isdigit((unsigned char) *s) && *s >= '5')
+       value++;
 
-   if (*s != '\0')
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                errmsg("invalid input syntax for type money: \"%s\"", str)));
+   /* adjust for less than required decimal places */
+   for (; dec < fpoint; dec++)
+       value *= 10;
+
+   /*
+    * should only be trailing digits followed by whitespace, right paren,
+    * or possibly a trailing minus sign
+    */
+   while (isdigit((unsigned char) *s))
+       s++;
+   while (*s)
+   {
+       if (isspace((unsigned char) *s) || *s == ')')
+           s++;
+       else if (strncmp(s, nsymbol, strlen(nsymbol)) == 0)
+       {
+           sgn = -1;
+           s += strlen(nsymbol);
+       }
+       else
+           ereport(ERROR,
+                   (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                    errmsg("invalid input syntax for type money: \"%s\"",
+                           str)));
+   }
 
    result = value * sgn;
 
@@ -200,25 +212,23 @@ cash_in(PG_FUNCTION_ARGS)
 
 
 /* cash_out()
- * Function to convert cash to a dollars and cents representation.
- * XXX HACK This code appears to assume US conventions for
- * positive-valued amounts. - tgl 97/04/14
+ * Function to convert cash to a dollars and cents representation, using
+ * the lc_monetary locale's formatting.
  */
 Datum
 cash_out(PG_FUNCTION_ARGS)
 {
    Cash        value = PG_GETARG_CASH(0);
    char       *result;
-   char        buf[CASH_BUFSZ];
-   int         minus = 0;
-   int         count = LAST_DIGIT;
-   int         point_pos;
-   int         comma_position = 0;
+   char        buf[128];
+   char       *bufptr;
+   bool        minus = false;
+   int         digit_pos;
    int         points,
                mon_group;
-   char        comma;
-   char       *csymbol,
-               dsymbol,
+   char        dsymbol;
+   const char *ssymbol,
+              *csymbol,
               *nsymbol;
    char        convention;
 
@@ -237,66 +247,79 @@ cash_out(PG_FUNCTION_ARGS)
    if (mon_group <= 0 || mon_group > 6)
        mon_group = 3;
 
-   comma = ((*lconvert->mon_thousands_sep != '\0') ? *lconvert->mon_thousands_sep : ',');
    convention = lconvert->n_sign_posn;
-   dsymbol = ((*lconvert->mon_decimal_point != '\0') ? *lconvert->mon_decimal_point : '.');
-   csymbol = ((*lconvert->currency_symbol != '\0') ? lconvert->currency_symbol : "$");
-   nsymbol = ((*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-");
-
-   point_pos = LAST_DIGIT - points;
 
-   /* allow more than three decimal points and separate them */
-   if (comma)
-   {
-       point_pos -= (points - 1) / mon_group;
-       comma_position = point_pos % (mon_group + 1);
-   }
+   /* we restrict dsymbol to be a single byte, but not the other symbols */
+   if (*lconvert->mon_decimal_point != '\0' &&
+       lconvert->mon_decimal_point[1] == '\0')
+       dsymbol = *lconvert->mon_decimal_point;
+   else
+       dsymbol = '.';
+   if (*lconvert->mon_thousands_sep != '\0')
+       ssymbol = lconvert->mon_thousands_sep;
+   else                        /* ssymbol should not equal dsymbol */
+       ssymbol = (dsymbol != ',') ? "," : ".";
+   csymbol = (*lconvert->currency_symbol != '\0') ? lconvert->currency_symbol : "$";
+   nsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
 
    /* we work with positive amounts and add the minus sign at the end */
    if (value < 0)
    {
-       minus = 1;
+       minus = true;
        value = -value;
    }
 
-   /* allow for trailing negative strings */
-   MemSet(buf, ' ', CASH_BUFSZ);
-   buf[TERMINATOR] = buf[LAST_PAREN] = '\0';
+   /* we build the result string right-to-left in buf[] */
+   bufptr = buf + sizeof(buf) - 1;
+   *bufptr = '\0';
 
-   while (value || count > (point_pos - 2))
+   /*
+    * Generate digits till there are no non-zero digits left and we emitted
+    * at least one to the left of the decimal point.  digit_pos is the
+    * current digit position, with zero as the digit just left of the decimal
+    * point, increasing to the right.
+    */
+   digit_pos = points;
+   do
    {
-       if (points && count == point_pos)
-           buf[count--] = dsymbol;
-       else if (comma && count % (mon_group + 1) == comma_position)
-           buf[count--] = comma;
+       if (points && digit_pos == 0)
+       {
+           /* insert decimal point */
+           *(--bufptr) = dsymbol;
+       }
+       else if (digit_pos < points && (digit_pos % mon_group) == 0)
+       {
+           /* insert thousands sep */
+           bufptr -= strlen(ssymbol);
+           memcpy(bufptr, ssymbol, strlen(ssymbol));
+       }
 
-       buf[count--] = ((unsigned int) value % 10) + '0';
+       *(--bufptr) = ((unsigned int) value % 10) + '0';
        value = ((unsigned int) value) / 10;
-   }
-
-   strncpy((buf + count - strlen(csymbol) + 1), csymbol, strlen(csymbol));
-   count -= strlen(csymbol) - 1;
+       digit_pos--;
+   } while (value || digit_pos >= 0);
 
-   if (buf[LAST_DIGIT] == ',')
-       buf[LAST_DIGIT] = buf[LAST_PAREN];
+   /* prepend csymbol */
+   bufptr -= strlen(csymbol);
+   memcpy(bufptr, csymbol, strlen(csymbol));
 
    /* see if we need to signify negative amount */
    if (minus)
    {
-       result = palloc(CASH_BUFSZ + 2 - count + strlen(nsymbol));
+       result = palloc(strlen(bufptr) + strlen(nsymbol) + 3);
 
        /* Position code of 0 means use parens */
        if (convention == 0)
-           sprintf(result, "(%s)", buf + count);
+           sprintf(result, "(%s)", bufptr);
        else if (convention == 2)
-           sprintf(result, "%s%s", buf + count, nsymbol);
+           sprintf(result, "%s%s", bufptr, nsymbol);
        else
-           sprintf(result, "%s%s", nsymbol, buf + count);
+           sprintf(result, "%s%s", nsymbol, bufptr);
    }
    else
    {
-       result = palloc(CASH_BUFSZ + 2 - count);
-       strcpy(result, buf + count);
+       /* just emit what we have */
+       result = pstrdup(bufptr);
    }
 
    PG_RETURN_CSTRING(result);