Re: [PATCH v3 3/4] tests: Avoid sprintf() with "%.6f"

2026-03-04 Thread Markus Armbruster
Akihiko Odaki  writes:

> A string that represents a double can be long if it is an exponentially
> large number.

I'd like us to spell out that this is cleanup, not a bug fix.  I suggest
to start with the warning.  Something like this:

tests: Clean up double comparisons to avoid compiler warning

To enable -Wformat-overflow=2, we need to clean up a couple of false
positives:



Now explain why they are false:

These buffers cannot actually overflow because the doubles are
between 0 and 31.0/3 inclusive.

Then justify the solution:

However, formatting doubles just to compare them is silly.  Compare
them directly instead.  To avoid potential rounding trouble, change
the numbers tested to be representable exactly in double.

> Signed-off-by: Akihiko Odaki 
> ---
>  tests/unit/test-qobject-input-visitor.c  | 8 ++--
>  tests/unit/test-qobject-output-visitor.c | 7 ++-
>  2 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/tests/unit/test-qobject-input-visitor.c 
> b/tests/unit/test-qobject-input-visitor.c
> index 84bdcdf702e0..beee11db4e47 100644
> --- a/tests/unit/test-qobject-input-visitor.c
> +++ b/tests/unit/test-qobject-input-visitor.c
> @@ -500,7 +500,7 @@ static void 
> test_visitor_in_list_struct(TestInputVisitorData *data,
>  g_string_append_printf(json, "'number': [");
>  sep = "";
>  for (i = 0; i < 32; i++) {
> -g_string_append_printf(json, "%s%f", sep, (double)i / 3);
> +g_string_append_printf(json, "%s%f", sep, (double)i / FLT_RADIX);
>  sep = ", ";
>  }
>  g_string_append_printf(json, "], ");
> @@ -583,11 +583,7 @@ static void 
> test_visitor_in_list_struct(TestInputVisitorData *data,
>  
>  i = 0;
>  for (num_list = arrs->number; num_list; num_list = num_list->next) {
> -char expected[32], actual[32];
> -
> -sprintf(expected, "%.6f", (double)i / 3);
> -sprintf(actual, "%.6f", num_list->value);
> -g_assert_cmpstr(expected, ==, actual);
> +g_assert_cmpfloat(num_list->value, ==, (double)i / FLT_RADIX);
>  i++;
>  }
>  
> diff --git a/tests/unit/test-qobject-output-visitor.c 
> b/tests/unit/test-qobject-output-visitor.c
> index 407ab9ed505a..3c47b28f6638 100644
> --- a/tests/unit/test-qobject-output-visitor.c
> +++ b/tests/unit/test-qobject-output-visitor.c
> @@ -538,7 +538,7 @@ static void 
> test_visitor_out_list_struct(TestOutputVisitorData *data,
>  }
>  
>  for (i = 31; i >= 0; i--) {
> -QAPI_LIST_PREPEND(arrs->number, (double)i / 3);
> +QAPI_LIST_PREPEND(arrs->number, (double)i / FLT_RADIX);
>  }
>  
>  for (i = 31; i >= 0; i--) {
> @@ -571,12 +571,9 @@ static void 
> test_visitor_out_list_struct(TestOutputVisitorData *data,
>  i = 0;
>  QLIST_FOREACH_ENTRY(qlist, e) {
>  QNum *qvalue = qobject_to(QNum, qlist_entry_obj(e));
> -char expected[32], actual[32];
>  
>  g_assert(qvalue);
> -sprintf(expected, "%.6f", (double)i / 3);
> -sprintf(actual, "%.6f", qnum_get_double(qvalue));
> -g_assert_cmpstr(actual, ==, expected);
> +g_assert_cmpfloat(qnum_get_double(qvalue), ==, (double)i / 
> FLT_RADIX);
>  i++;
>  }

With an improved commit message
Reviewed-by: Markus Armbruster 




[PATCH v3 3/4] tests: Avoid sprintf() with "%.6f"

2026-03-03 Thread Akihiko Odaki
A string that represents a double can be long if it is an exponentially
large number.

Signed-off-by: Akihiko Odaki 
---
 tests/unit/test-qobject-input-visitor.c  | 8 ++--
 tests/unit/test-qobject-output-visitor.c | 7 ++-
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/tests/unit/test-qobject-input-visitor.c 
b/tests/unit/test-qobject-input-visitor.c
index 84bdcdf702e0..beee11db4e47 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -500,7 +500,7 @@ static void 
test_visitor_in_list_struct(TestInputVisitorData *data,
 g_string_append_printf(json, "'number': [");
 sep = "";
 for (i = 0; i < 32; i++) {
-g_string_append_printf(json, "%s%f", sep, (double)i / 3);
+g_string_append_printf(json, "%s%f", sep, (double)i / FLT_RADIX);
 sep = ", ";
 }
 g_string_append_printf(json, "], ");
@@ -583,11 +583,7 @@ static void 
test_visitor_in_list_struct(TestInputVisitorData *data,
 
 i = 0;
 for (num_list = arrs->number; num_list; num_list = num_list->next) {
-char expected[32], actual[32];
-
-sprintf(expected, "%.6f", (double)i / 3);
-sprintf(actual, "%.6f", num_list->value);
-g_assert_cmpstr(expected, ==, actual);
+g_assert_cmpfloat(num_list->value, ==, (double)i / FLT_RADIX);
 i++;
 }
 
diff --git a/tests/unit/test-qobject-output-visitor.c 
b/tests/unit/test-qobject-output-visitor.c
index 407ab9ed505a..3c47b28f6638 100644
--- a/tests/unit/test-qobject-output-visitor.c
+++ b/tests/unit/test-qobject-output-visitor.c
@@ -538,7 +538,7 @@ static void 
test_visitor_out_list_struct(TestOutputVisitorData *data,
 }
 
 for (i = 31; i >= 0; i--) {
-QAPI_LIST_PREPEND(arrs->number, (double)i / 3);
+QAPI_LIST_PREPEND(arrs->number, (double)i / FLT_RADIX);
 }
 
 for (i = 31; i >= 0; i--) {
@@ -571,12 +571,9 @@ static void 
test_visitor_out_list_struct(TestOutputVisitorData *data,
 i = 0;
 QLIST_FOREACH_ENTRY(qlist, e) {
 QNum *qvalue = qobject_to(QNum, qlist_entry_obj(e));
-char expected[32], actual[32];
 
 g_assert(qvalue);
-sprintf(expected, "%.6f", (double)i / 3);
-sprintf(actual, "%.6f", qnum_get_double(qvalue));
-g_assert_cmpstr(actual, ==, expected);
+g_assert_cmpfloat(qnum_get_double(qvalue), ==, (double)i / FLT_RADIX);
 i++;
 }
 

-- 
2.53.0