From 38b138cd0ab3a83b248ecb3e5a18e4329586eddb Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Mon, 2 Jan 2023 21:57:42 -0800
Subject: [PATCH] Fixed rounding of floating point values in snprintf
---
src/stdlib/SDL_string.c | 135 +++++++++++++++++++++--------------
test/testautomation_stdlib.c | 51 ++++++++++---
2 files changed, 120 insertions(+), 66 deletions(-)
diff --git a/src/stdlib/SDL_string.c b/src/stdlib/SDL_string.c
index 047dd572749c..7cb2a41d3c91 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 (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 */
diff --git a/test/testautomation_stdlib.c b/test/testautomation_stdlib.c
index 2d2ed0fdabe4..2460d9382522 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);
-
- 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");