Fixed rounding of floating point values in snprintf

src/stdlib/SDL_string.c

@@ -1621,44 +1621,97 @@ SDL_PrintUnsignedLongLong(char *text, size_t maxlen, SDL_FormatInfo *info, Uint6
 }
 
 static size_t
-SDL_PrintFloat(char *text, size_t maxlen, SDL_FormatInfo *info, double arg)
+SDL_PrintFloat(char *text, size_t maxlen, SDL_FormatInfo *info, double arg, SDL_bool g)
 {
+    char num[327];
     size_t length = 0;
+    size_t integer_length;
+    int precision = info->precision;
 
     /* This isn't especially accurate, but hey, it's easy. :) */
-    unsigned long value;
+    Uint64 value;
 
     if (arg < 0) {
-        if (length < maxlen) {
-            text[length] = '-';
-        }
-        ++length;
+        num[length++] = '-';
         arg = -arg;
     } else if (info->force_sign) {
-        if (length < maxlen) {
-            text[length] = '+';
-        }
-        ++length;
+        num[length++] = '+';
     }
-    value = (unsigned long)arg;
-    length += SDL_PrintUnsignedLong(TEXT_AND_LEN_ARGS, NULL, value);
+    value = (Uint64)arg;
+    integer_length = SDL_PrintUnsignedLongLong(&num[length], sizeof(num) - length, NULL, value);
+    length += integer_length;
     arg -= value;
-    if (info->precision < 0) {
-        info->precision = 6;
-    }
-    if (info->force_type || info->precision > 0) {
-        int mult = 10;
-        if (length < maxlen) {
-            text[length] = '.';
+    if (precision < 0) {
+        precision = 6;
+    }
+    if (g) {
+        /* The precision includes the integer portion */
+        precision -= SDL_min((int)integer_length, precision);
+    }
+    if (info->force_type || precision > 0) {
+        const char decimal_separator = '.';
+        double integer_value;
+
+        SDL_assert(length < sizeof(num));
+        num[length++] = decimal_separator;
+        while (precision > 1) {
+            arg *= 10.0;
+            arg = SDL_modf(arg, &integer_value);
+            SDL_assert(length < sizeof(num));
+            num[length++] = '0' + (int)integer_value;
+            --precision;
         }
-        ++length;
-        while (info->precision-- > 0) {
-            value = (unsigned long)(arg * mult);
-            length += SDL_PrintUnsignedLong(TEXT_AND_LEN_ARGS, NULL, value);
-            arg -= (double)value / mult;
-            mult *= 10;
+        if (precision == 1) {
+            arg *= 10.0;
+            integer_value = SDL_round(arg);
+            if (integer_value == 10.0) {
+                /* Carry the one... */
+                size_t i;
+
+                for (i = length; i--; ) {
+                    if (num[i] == decimal_separator) {
+                        continue;
+                    }
+                    if (num[i] == '9') {
+                        num[i] = '0';
+                    } else {
+                        ++num[i];
+                        break;
+                    }
+                }
+                if (i == 0 || num[i] == '-' || num[i] == '+') {
+                    SDL_memmove(&num[i+1], &num[i], length - i);
+                    num[i] = '1';
+                    ++length;
+                }
+                SDL_assert(length < sizeof(num));
+                num[length++] = '0';
+            } else {
+                SDL_assert(length < sizeof(num));
+                num[length++] = '0' + (int)integer_value;
+            }
+        }
+
+        if (g) {
+            /* Trim trailing zeroes and decimal separator */
+            size_t i;
+
+            for (i = length - 1; num[i] != decimal_separator; --i) {
+                if (num[i] == '0') {
+                    --length;
+                } else {
+                    break;
+                }
+            }
+            if (num[i] == decimal_separator) {
+                --length;
+            }
         }
     }
+    num[length] = '\0';
+
+    info->precision = -1;
+    length = SDL_PrintString(text, maxlen, info, num);
 
     if (info->width > 0 && (size_t)info->width > length) {
         const char fill = info->pad_zeroes ? '0' : ' ';
@@ -1671,30 +1724,6 @@ SDL_PrintFloat(char *text, size_t maxlen, SDL_FormatInfo *info, double arg)
         SDL_memset(text, fill, filllen);
         length += width;
     }
-
-    return length;
-}
-
-static size_t
-SDL_TrimTrailingFractionalZeroes(char *text, size_t start, size_t length)
-{
-    size_t i, j;
-
-    for (i = start; i < length; ++i) {
-        if (text[i] == '.' || text[i] == ',') {
-            for (j = length - 1; j > i; --j) {
-                if (text[j] == '0') {
-                    --length;
-                } else {
-                    break;
-                }
-            }
-            if (j == i) {
-                --length;
-            }
-            break;
-        }
-    }
     return length;
 }
 
@@ -1876,17 +1905,13 @@ int SDL_vsnprintf(SDL_OUT_Z_CAP(maxlen) char *text, size_t maxlen, const char *f
                     done = SDL_TRUE;
                     break;
                 case 'f':
-                    length += SDL_PrintFloat(TEXT_AND_LEN_ARGS, &info, va_arg(ap, double));
+                    length += SDL_PrintFloat(TEXT_AND_LEN_ARGS, &info, va_arg(ap, double), SDL_FALSE);
                     done = SDL_TRUE;
                     break;
                 case 'g':
-                {
-                    size_t starting_length = length;
-                    length += SDL_PrintFloat(TEXT_AND_LEN_ARGS, &info, va_arg(ap, double));
-                    length = SDL_TrimTrailingFractionalZeroes(text, starting_length, length);
+                    length += SDL_PrintFloat(TEXT_AND_LEN_ARGS, &info, va_arg(ap, double), SDL_TRUE);
                     done = SDL_TRUE;
                     break;
-                }
                 case 'S':
                 {
                     /* In practice this is used on Windows for WCHAR strings */

test/testautomation_stdlib.c

@@ -153,17 +153,46 @@ int stdlib_snprintf(void *arg)
     SDLTest_AssertCheck(SDL_strcmp(text, expected) == 0, "Check text, expected: '%s', got: '%s'", expected, text);
     SDLTest_AssertCheck(result == 6, "Check result value, expected: 6, got: %d", result);
 
-    result = SDL_snprintf(text, sizeof(text), "%g", 100.0);
-    expected = "100";
-    SDLTest_AssertPass("Call to SDL_snprintf(\"%%g\", 100.0)");
-    SDLTest_AssertCheck(SDL_strcmp(text, expected) == 0, "Check text, expected: '%s', got: '%s'", expected, text);
-    SDLTest_AssertCheck(result == 3, "Check result value, expected: 3, got: %d", result);
-
-    result = SDL_snprintf(text, sizeof(text), "%g", 100.75);
-    expected = "100.75";
-    SDLTest_AssertPass("Call to SDL_snprintf(\"%%g\", 100.75)");
-    SDLTest_AssertCheck(SDL_strcmp(text, expected) == 0, "Check text, expected: '%s', got: '%s'", expected, text);
-    SDLTest_AssertCheck(result == 6, "Check result value, expected: 6, got: %d", result);
+    {
+        static struct
+        {
+            float value;
+            const char *expected_f;
+            const char *expected_g;
+        } f_and_g_test_cases[] = {
+            { 100.0f, "100.000000", "100" },
+            { -100.0f, "-100.000000", "-100" },
+            { 100.75f, "100.750000", "100.75" },
+            { -100.75f, "-100.750000", "-100.75" },
+            { ((100 * 60 * 1000) / 1001) / 100.0f, "59.939999", "59.94" },
+            { -((100 * 60 * 1000) / 1001) / 100.0f, "-59.939999", "-59.94" },
+            { ((100 * 120 * 1000) / 1001) / 100.0f, "119.879997", "119.88" },
+            { -((100 * 120 * 1000) / 1001) / 100.0f, "-119.879997", "-119.88" },
+            { 9.9999999f, "10.000000", "10" },
+            { -9.9999999f, "-10.000000", "-10" },
+        };
+        int i;
+
+        for (i = 0; i < SDL_arraysize(f_and_g_test_cases); ++i) {
+            float value = f_and_g_test_cases[i].value;
+
+            result = SDL_snprintf(text, sizeof(text), "%f", value);
+            predicted = SDL_snprintf(NULL, 0, "%f", value);
+            expected = f_and_g_test_cases[i].expected_f;
+            SDLTest_AssertPass("Call to SDL_snprintf(\"%%f\", %g)", value);
+            SDLTest_AssertCheck(SDL_strcmp(text, expected) == 0, "Check text, expected: '%s', got: '%s'", expected, text);
+            SDLTest_AssertCheck(result == SDL_strlen(expected), "Check result value, expected: %d, got: %d", SDL_strlen(expected), result);
+            SDLTest_AssertCheck(predicted == result, "Check predicted value, expected: %d, got: %d", result, predicted);
+
+            result = SDL_snprintf(text, sizeof(text), "%g", value);
+            predicted = SDL_snprintf(NULL, 0, "%g", value);
+            expected = f_and_g_test_cases[i].expected_g;
+            SDLTest_AssertPass("Call to SDL_snprintf(\"%%g\", %g)", value);
+            SDLTest_AssertCheck(SDL_strcmp(text, expected) == 0, "Check text, expected: '%s', got: '%s'", expected, text);
+            SDLTest_AssertCheck(result == SDL_strlen(expected), "Check result value, expected: %d, got: %d", SDL_strlen(expected), result);
+            SDLTest_AssertCheck(predicted == result, "Check predicted value, expected: %d, got: %d", result, predicted);
+        }
+    }
 
     size = 64;
     result = SDL_snprintf(text, sizeof(text), "%zu %s", size, "test");