From 38b138cd0ab3a83b248ecb3e5a18e4329586eddb Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 2 Jan 2023 21:57:42 -0800 Subject: [PATCH] Fixed rounding of floating point values in snprintf --- src/stdlib/SDL_string.c | 133 +++++++++++++++++++++-------------- test/testautomation_stdlib.c | 49 ++++++++++--- 2 files changed, 118 insertions(+), 64 deletions(-) diff --git a/src/stdlib/SDL_string.c b/src/stdlib/SDL_string.c index 047dd5727..7cb2a41d3 100644 --- a/src/stdlib/SDL_string.c +++ b/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 (precision < 0) { + precision = 6; } - if (info->force_type || info->precision > 0) { - int mult = 10; - if (length < maxlen) { - text[length] = '.'; + 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 */ diff --git a/test/testautomation_stdlib.c b/test/testautomation_stdlib.c index 2d2ed0fda..2460d9382 100644 --- a/test/testautomation_stdlib.c +++ b/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); + { + 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; - 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); + 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");