Merge branch 'saleyn/float_to_list_2/OTP-10837'

* saleyn/float_to_list_2/OTP-10837:
  Fix memory leak in error case
  Use macros instead of constants
  float_to_list/2 changed rounding and cosmetic cleanup

1 files changed, 111 insertions, 84 deletions

diff --git a/erts/emulator/sys/common/erl_sys_common_misc.c b/erts/emulator/sys/common/erl_sys_common_misc.c
index a0cd4cd10a..0b31c125e5 100644
--- a/erts/emulator/sys/common/erl_sys_common_misc.c
+++ b/erts/emulator/sys/common/erl_sys_common_misc.c
@@ -114,9 +114,9 @@ int erts_get_native_filename_encoding(void)
 }
 
 /* For internal use by sys_double_to_chars_fast() */
-static char* float_first_trailing_zero(char* p)
+static char* find_first_trailing_zero(char* p)
 {
-    for (--p; *p == '0' && *(p-1) == '0'; --p);
+    for (; *(p-1) == '0'; --p);
     if (*(p-1) == '.') ++p;
     return p;
 }
@@ -127,34 +127,83 @@ sys_double_to_chars(double fp, char *buffer, size_t buffer_size)
     return sys_double_to_chars_ext(fp, buffer, buffer_size, SYS_DEFAULT_FLOAT_DECIMALS);
 }
 
+/* Convert float to string using fixed point notation.
+ *   decimals must be >= 0
+ *   if compact != 0, the trailing 0's will be truncated
+ */
 int
-sys_double_to_chars_fast(double f, char *outbuf, int maxlen, int decimals, int compact)
+sys_double_to_chars_fast(double f, char *buffer, int buffer_size, int decimals,
+			 int compact)
 {
-    enum {
-          FRAC_SIZE  = 52
-        , EXP_SIZE   = 11
-        , EXP_MASK   = (1ll << EXP_SIZE) - 1
-        , FRAC_MASK  = (1ll << FRAC_SIZE) - 1
-        , FRAC_MASK2 = (1ll << (FRAC_SIZE + 1)) - 1
-        , MAX_FLOAT  = 1ll << (FRAC_SIZE+1)
+    /* Note that some C compilers don't support "static const" propagation
+     * so we use a defines */
+    #define SYS_DOUBLE_RND_CONST 0.55555555555555555
+    #define FRAC_SIZE            52
+    #define EXP_SIZE             11
+    #define EXP_MASK             ((1ll << EXP_SIZE) - 1)
+    #define MAX_DECIMALS         (sizeof(cs_sys_double_pow10) \
+				   / sizeof(cs_sys_double_pow10[0]))
+    #define FRAC_MASK            ((1ll << FRAC_SIZE) - 1)
+    #define FRAC_MASK2           ((1ll << (FRAC_SIZE + 1)) - 1)
+    #define MAX_FLOAT            (1ll << (FRAC_SIZE+1))
+
+    static const double cs_sys_double_pow10[] = {
+        SYS_DOUBLE_RND_CONST / 1ll,
+        SYS_DOUBLE_RND_CONST / 10ll,
+        SYS_DOUBLE_RND_CONST / 100ll,
+        SYS_DOUBLE_RND_CONST / 1000ll,
+        SYS_DOUBLE_RND_CONST / 10000ll,
+        SYS_DOUBLE_RND_CONST / 100000ll,
+        SYS_DOUBLE_RND_CONST / 1000000ll,
+        SYS_DOUBLE_RND_CONST / 10000000ll,
+        SYS_DOUBLE_RND_CONST / 100000000ll,
+        SYS_DOUBLE_RND_CONST / 1000000000ll,
+        SYS_DOUBLE_RND_CONST / 10000000000ll,
+        SYS_DOUBLE_RND_CONST / 100000000000ll,
+        SYS_DOUBLE_RND_CONST / 1000000000000ll,
+        SYS_DOUBLE_RND_CONST / 10000000000000ll,
+        SYS_DOUBLE_RND_CONST / 100000000000000ll,
+        SYS_DOUBLE_RND_CONST / 1000000000000000ll,
+        SYS_DOUBLE_RND_CONST / 10000000000000000ll,
+        SYS_DOUBLE_RND_CONST / 100000000000000000ll,
+        SYS_DOUBLE_RND_CONST / 1000000000000000000ll
     };
 
-    long long mantissa, int_part, int_part2, frac_part;
+    long long mantissa, int_part = 0, frac_part = 0;
     short exp;
-    int sign, i, n, m, max;
-    double absf;
+    int max;
+    int neg;
+    double fr;
     union { long long L; double F; } x;
-    char c, *p = outbuf;
-    int digit, roundup;
+    char *p = buffer;
+
+    if (decimals < 0)
+        return -1;
 
-    x.F = f;
+    /* Round the number to given decimal places. The number of 5's in the
+     * SYS_DOUBLE_RND_CONST constant is chosen such that adding any more 5's doesn't
+     * change the double precision of the number, i.e.:
+     * 1> term_to_binary(0.55555555555555555, [{minor_version, 1}]).
+     * <<131,70,63,225,199,28,113,199,28,114>>
+     * 2> term_to_binary(0.5555555555555555555, [{minor_version, 1}]).
+     * <<131,70,63,225,199,28,113,199,28,114>>
+     */
+    if (f >= 0) {
+        neg = 0;
+        fr  = decimals < MAX_DECIMALS ? (f + cs_sys_double_pow10[decimals]) : f;
+        x.F = fr;
+    } else {
+        neg = 1;
+        fr  = decimals < MAX_DECIMALS ? (f - cs_sys_double_pow10[decimals]) : f;
+        x.F = -fr;
+    }
 
     exp      = (x.L >> FRAC_SIZE) & EXP_MASK;
     mantissa = x.L & FRAC_MASK;
-    sign     = x.L >= 0 ? 1 : -1;
+
     if (exp == EXP_MASK) {
         if (mantissa == 0) {
-            if (sign == -1)
+            if (neg)
                 *p++ = '-';
             *p++ = 'i';
             *p++ = 'n';
@@ -165,101 +214,79 @@ sys_double_to_chars_fast(double f, char *outbuf, int maxlen, int decimals, int c
             *p++ = 'n';
         }
         *p = '\0';
-        return p - outbuf;
+        return p - buffer;
     }
 
     exp      -= EXP_MASK >> 1;
     mantissa |= (1ll << FRAC_SIZE);
-    frac_part = 0;
-    int_part  = 0;
-    absf      = f * sign;
-
-    /* Don't bother with optimizing too large numbers and decimals */
-    if (absf > MAX_FLOAT || decimals > maxlen-17) {
-        int len = erts_snprintf(outbuf, maxlen, "%.*f", decimals, f);
-        if (len >= maxlen)
+
+    /* Don't bother with optimizing too large numbers or too large precision */
+    if (x.F > MAX_FLOAT || decimals >= MAX_DECIMALS) {
+        int len = erts_snprintf(buffer, buffer_size, "%.*f", decimals, f);
+        char* p = buffer + len;
+        if (len >= buffer_size)
             return -1;
-        p = outbuf + len;
         /* Delete trailing zeroes */
         if (compact)
-            p = float_first_trailing_zero(outbuf + len);
+            p = find_first_trailing_zero(p);
         *p = '\0';
-        return p - outbuf;
-    }
-
-    if (exp >= FRAC_SIZE)
+        return p - buffer;
+    } else if (exp >= FRAC_SIZE) {
         int_part  = mantissa << (exp - FRAC_SIZE);
-    else if (exp >= 0) {
+    } else if (exp >= 0) {
         int_part  = mantissa >> (FRAC_SIZE - exp);
         frac_part = (mantissa << (exp + 1)) & FRAC_MASK2;
-    }
-    else /* if (exp < 0) */
+    } else /* if (exp < 0) */ {
         frac_part = (mantissa & FRAC_MASK2) >> -(exp + 1);
+    }
 
-    if (int_part == 0) {
-        if (sign == -1)
+    if (!int_part) {
+        if (neg)
             *p++ = '-';
         *p++ = '0';
     } else {
-        int ret;
+        int ret, i, n;
         while (int_part != 0) {
-            int_part2 = int_part / 10;
-            *p++ = (char)(int_part - ((int_part2 << 3) + (int_part2 << 1)) + '0');
-            int_part = int_part2;
+            long long j = int_part / 10;
+            *p++ = (char)(int_part - ((j << 3) + (j << 1)) + '0');
+            int_part = j;
         }
-        if (sign == -1)
+        if (neg)
             *p++ = '-';
         /* Reverse string */
-        ret = p - outbuf;
+        ret = p - buffer;
         for (i = 0, n = ret/2; i < n; i++) {
-            int j = ret - i - 1;
-            c = outbuf[i];
-            outbuf[i] = outbuf[j];
-            outbuf[j] = c;
+            int  j = ret - i - 1;
+            char c = buffer[i];
+            buffer[i] = buffer[j];
+            buffer[j] = c;
         }
     }
-    if (decimals != 0)
+
+    if (decimals > 0) {
+        int i;
         *p++ = '.';
 
-    max = maxlen - (p - outbuf) - 1 /* leave room for trailing '\0' */;
-    if (max > decimals)
+        max = buffer_size - (p - buffer) - 1 /* leave room for trailing '\0' */;
+
+        if (decimals > max)
+            return -1;  /* the number is not large enough to fit in the buffer */
+
         max = decimals;
-    for (m = 0; m < max; m++) {
-        /* frac_part *= 10; */
-        frac_part = (frac_part << 3) + (frac_part << 1);
 
-        *p++ = (char)((frac_part >> (FRAC_SIZE + 1)) + '0');
-        frac_part &= FRAC_MASK2;
-    }
+        for (i = 0; i < max; i++) {
+            /* frac_part *= 10; */
+            frac_part = (frac_part << 3) + (frac_part << 1);
 
-    roundup = 0;
-    /* Rounding - look at the next digit */
-    frac_part = (frac_part << 3) + (frac_part << 1);
-    digit = (frac_part >> (FRAC_SIZE + 1));
-    if (digit > 5)
-        roundup = 1;
-    else if (digit == 5) {
-        frac_part &= FRAC_MASK2;
-        if (frac_part != 0) roundup = 1;
-    }
-    if (roundup) {
-        char d;
-        int pos = p - outbuf - 1;
-        do {
-            d = outbuf[pos];
-            if (d == '-') break;
-            if (d == '.') continue;
-            if (++d != ':') {
-                outbuf[pos] = d;
-                break;
-            }
-            outbuf[pos] = '0';
-        } while (--pos);
+            *p++ = (char)((frac_part >> (FRAC_SIZE + 1)) + '0');
+            frac_part &= FRAC_MASK2;
+        }
+
+        /* Delete trailing zeroes */
+        if (compact)
+            p = find_first_trailing_zero(p);
     }
 
-    /* Delete trailing zeroes */
-    if (compact && *(p - 1) == '0')
-        p = float_first_trailing_zero(--p);
     *p = '\0';
-    return p - outbuf;
+    return p - buffer;
 }