From: Tom Lane Date: Sat, 29 Oct 2011 18:31:15 +0000 (-0400) Subject: Fix assorted bogosities in cash_in() and cash_out(). X-Git-Tag: REL8_2_23~11 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5b297de51b014e510c59d0bdffe8130afccabc3c;p=thirdparty%2Fpostgresql.git Fix assorted bogosities in cash_in() and cash_out(). 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. --- diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index ca0a0453683..0cab38b01c2 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -26,13 +26,6 @@ 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);