Replace static bufs with a StringInfo in cash_words()

For clarity. The code was correct, and the buffer was large enough,
but string manipulation with no bounds checking is scary.

This incurs an extra palloc+pfree to every call, but in quick
performance testing, it doesn't seem to be significant.

Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/7f86e06a-98c5-4ce3-8ec9-3885c8de0358@iki.fi

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index b20c358486dc25af1216191cdc52fa4fae953d45..ec3c08acfc2df970df90b15265ee57965177e131 100644 (file)
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -35,10 +35,9 @@
  * Private routines
  ************************************************************************/
 
-static const char *
-num_word(Cash value)
+static void
+append_num_word(StringInfo buf, Cash value)
 {
-       static char buf[128];
        static const char *const small[] = {
                "zero", "one", "two", "three", "four", "five", "six", "seven",
                "eight", "nine", "ten", "eleven", "twelve", "thirteen", "fourteen",
@@ -50,13 +49,16 @@ num_word(Cash value)
 
        /* deal with the simple cases first */
        if (value <= 20)
-               return small[value];
+       {
+               appendStringInfoString(buf, small[value]);
+               return;
+       }
 
        /* is it an even multiple of 100? */
        if (!tu)
        {
-               sprintf(buf, "%s hundred", small[value / 100]);
-               return buf;
+               appendStringInfo(buf, "%s hundred", small[value / 100]);
+               return;
        }
 
        /* more than 99? */
@@ -64,27 +66,25 @@ num_word(Cash value)
        {
                /* is it an even multiple of 10 other than 10? */
                if (value % 10 == 0 && tu > 10)
-                       sprintf(buf, "%s hundred %s",
-                                       small[value / 100], big[tu / 10]);
+                       appendStringInfo(buf, "%s hundred %s",
+                                                        small[value / 100], big[tu / 10]);
                else if (tu < 20)
-                       sprintf(buf, "%s hundred and %s",
-                                       small[value / 100], small[tu]);
+                       appendStringInfo(buf, "%s hundred and %s",
+                                                        small[value / 100], small[tu]);
                else
-                       sprintf(buf, "%s hundred %s %s",
-                                       small[value / 100], big[tu / 10], small[tu % 10]);
+                       appendStringInfo(buf, "%s hundred %s %s",
+                                                        small[value / 100], big[tu / 10], small[tu % 10]);
        }
        else
        {
                /* is it an even multiple of 10 other than 10? */
                if (value % 10 == 0 && tu > 10)
-                       sprintf(buf, "%s", big[tu / 10]);
+                       appendStringInfoString(buf, big[tu / 10]);
                else if (tu < 20)
-                       sprintf(buf, "%s", small[tu]);
+                       appendStringInfoString(buf, small[tu]);
                else
-                       sprintf(buf, "%s %s", big[tu / 10], small[tu % 10]);
+                       appendStringInfo(buf, "%s %s", big[tu / 10], small[tu % 10]);
        }
-
-       return buf;
 }                                                              /* num_word() */
 
 static inline Cash
@@ -960,8 +960,9 @@ cash_words(PG_FUNCTION_ARGS)
 {
        Cash            value = PG_GETARG_CASH(0);
        uint64          val;
-       char            buf[256];
-       char       *p = buf;
+       StringInfoData buf;
+       text       *res;
+       Cash            dollars;
        Cash            m0;
        Cash            m1;
        Cash            m2;
@@ -970,19 +971,19 @@ cash_words(PG_FUNCTION_ARGS)
        Cash            m5;
        Cash            m6;
 
+       initStringInfo(&buf);
+
        /* work with positive numbers */
        if (value < 0)
        {
                value = -value;
-               strcpy(buf, "minus ");
-               p += 6;
+               appendStringInfoString(&buf, "minus ");
        }
-       else
-               buf[0] = '\0';
 
        /* Now treat as unsigned, to avoid trouble at INT_MIN */
        val = (uint64) value;
 
+       dollars = val / INT64CONST(100);
        m0 = val % INT64CONST(100); /* cents */
        m1 = (val / INT64CONST(100)) % 1000;    /* hundreds */
        m2 = (val / INT64CONST(100000)) % 1000; /* thousands */
@@ -993,49 +994,51 @@ cash_words(PG_FUNCTION_ARGS)
 
        if (m6)
        {
-               strcat(buf, num_word(m6));
-               strcat(buf, " quadrillion ");
+               append_num_word(&buf, m6);
+               appendStringInfoString(&buf, " quadrillion ");
        }
 
        if (m5)
        {
-               strcat(buf, num_word(m5));
-               strcat(buf, " trillion ");
+               append_num_word(&buf, m5);
+               appendStringInfoString(&buf, " trillion ");
        }
 
        if (m4)
        {
-               strcat(buf, num_word(m4));
-               strcat(buf, " billion ");
+               append_num_word(&buf, m4);
+               appendStringInfoString(&buf, " billion ");
        }
 
        if (m3)
        {
-               strcat(buf, num_word(m3));
-               strcat(buf, " million ");
+               append_num_word(&buf, m3);
+               appendStringInfoString(&buf, " million ");
        }
 
        if (m2)
        {
-               strcat(buf, num_word(m2));
-               strcat(buf, " thousand ");
+               append_num_word(&buf, m2);
+               appendStringInfoString(&buf, " thousand ");
        }
 
        if (m1)
-               strcat(buf, num_word(m1));
+               append_num_word(&buf, m1);
 
-       if (!*p)
-               strcat(buf, "zero");
+       if (dollars == 0)
+               appendStringInfoString(&buf, "zero");
 
-       strcat(buf, (val / 100) == 1 ? " dollar and " : " dollars and ");
-       strcat(buf, num_word(m0));
-       strcat(buf, m0 == 1 ? " cent" : " cents");
+       appendStringInfoString(&buf, dollars == 1 ? " dollar and " : " dollars and ");
+       append_num_word(&buf, m0);
+       appendStringInfoString(&buf, m0 == 1 ? " cent" : " cents");
 
        /* capitalize output */
-       buf[0] = pg_toupper((unsigned char) buf[0]);
+       buf.data[0] = pg_toupper((unsigned char) buf.data[0]);
 
        /* return as text datum */
-       PG_RETURN_TEXT_P(cstring_to_text(buf));
+       res = cstring_to_text_with_len(buf.data, buf.len);
+       pfree(buf.data);
+       PG_RETURN_TEXT_P(res);
 }