[PATCH 0/2] tests/qtest: Avoid char arrays in some Arm tests

[PATCH 0/2] tests/qtest: Avoid char arrays in some Arm tests

[Peter Maydell]

This patchset fixes a couple of problems reported by Fabiano Rosas
with gcc 7.5.0 now we have turned on -Wformat-overflow=2. In both
cases the test is using sprintf() into a fixed-size char array, and
it can't actually overflow but gcc doesn't realize that. We prefer
to use g_strdup_printf() for this kind of thing anyway, as it is
much less error-prone, so switch to that.

thanks
-- PMM

Peter Maydell (2):
  tests/qtest/ast2700-sgpio-test: Use g_strdup_printf() instead of char
    arrays
  tests/qtest/arm-cpu-features: Use g_strdup_printf() instead of char
    arrays

 tests/qtest/arm-cpu-features.c   | 24 +++++++++++++++---------
 tests/qtest/ast2700-sgpio-test.c | 24 ++++++++++++------------
 2 files changed, 27 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH 1/2] tests/qtest/ast2700-sgpio-test: Use g_strdup_printf() instead of char arrays

[Peter Maydell]

Older versions of gcc with -Wformat-overflow=2 don't like the usage of
fixed size char arrays in this test; gcc 7.5.0 (SUSE Linux) says:

../tests/qtest/ast2700-sgpio-test.c: In function ‘test_output_pins’:
../tests/qtest/ast2700-sgpio-test.c:27:33: error: ‘sprintf’ may write a
terminating nul past the end of the destination
[-Werror=format-overflow=]

../tests/qtest/ast2700-sgpio-test.c: In function ‘test_irq_level_high’:
../tests/qtest/ast2700-sgpio-test.c:85:33: error: ‘sprintf’ may write a
terminating nul past the end of the destination
[-Werror=format-overflow=]

These can't actually happen because of the limited size of the values
being substituted in.  However rather than require readers to check
whether the arrays really have been declared large enough, we prefer
to use g_strdup_printf() for this kind of string work.

Reported-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/ast2700-sgpio-test.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/qtest/ast2700-sgpio-test.c b/tests/qtest/ast2700-sgpio-test.c
index 56c54cca9b..529408ca8f 100644
--- a/tests/qtest/ast2700-sgpio-test.c
+++ b/tests/qtest/ast2700-sgpio-test.c
@@ -18,14 +18,14 @@
 static void test_output_pins(const char *machine, const uint32_t base, int idx)
 {
     QTestState *s = qtest_init(machine);
-    char name[16];
-    char qom_path[64];
     uint32_t offset = 0;
     uint32_t value = 0;
     for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
         /* Odd index is output port */
-        sprintf(name, "sgpio%03d", i * 2 + 1);
-        sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+        g_autofree const char *name = g_strdup_printf("sgpio%03d", i * 2 + 1);
+        g_autofree const char *qom_path
+            = g_strdup_printf("/machine/soc/sgpio[%d]", idx);
+
         offset = base + (R_SGPIO_0_CONTROL + i) * 4;
         /* set serial output */
         qtest_writel(s, offset, 0x00000001);
@@ -45,14 +45,14 @@ static void test_output_pins(const char *machine, const uint32_t base, int idx)
 static void test_input_pins(const char *machine, const uint32_t base, int idx)
 {
     QTestState *s = qtest_init(machine);
-    char name[16];
-    char qom_path[64];
     uint32_t offset = 0;
     uint32_t value = 0;
     for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
         /* Even index is input port */
-        sprintf(name, "sgpio%03d", i * 2);
-        sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+        g_autofree const char *name = g_strdup_printf("sgpio%03d", i * 2);
+        g_autofree const char *qom_path
+            = g_strdup_printf("/machine/soc/sgpio[%d]", idx);
+
         offset = base + (R_SGPIO_0_CONTROL + i) * 4;
         /* set serial input */
         qtest_qom_set_bool(s, qom_path, name, true);
@@ -73,8 +73,6 @@ static void test_irq_level_high(const char *machine,
                                 const uint32_t base, int idx)
 {
     QTestState *s = qtest_init(machine);
-    char name[16];
-    char qom_path[64];
     uint32_t ctrl_offset = 0;
     uint32_t int_offset = 0;
     uint32_t int_reg_idx = 0;
@@ -82,8 +80,10 @@ static void test_irq_level_high(const char *machine,
     uint32_t value = 0;
     for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
         /* Even index is input port */
-        sprintf(name, "sgpio%03d", i * 2);
-        sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+        g_autofree const char *name = g_strdup_printf("sgpio%03d", i * 2);
+        g_autofree const char *qom_path =
+            g_strdup_printf("/machine/soc/sgpio[%d]", idx);
+
         int_reg_idx = i / 32;
         int_bit_idx = i % 32;
         int_offset = base + (R_SGPIO_INT_STATUS_0 + int_reg_idx) * 4;
-- 
2.43.0

[PATCH 2/2] tests/qtest/arm-cpu-features: Use g_strdup_printf() instead of char arrays

[Peter Maydell]

Older versions of gcc with -Wformat-overflow=2 don't like the usage of
fixed size char arrays in this test; gcc 7.5.0 (SUSE Linux) says:

../tests/qtest/arm-cpu-features.c: In function ‘test_query_cpu_model_expansion_kvm’:
../tests/qtest/arm-cpu-features.c:578:35: error: ‘%u’ directive writing
between 1 and 10 bytes into a region of size 5
[-Werror=format-overflow=]

These can't actually happen because of the limited size of the values
being substituted in.  However rather than require readers to check
whether the arrays really have been declared large enough, we prefer
to use g_strdup_printf() for this kind of string work.

Reported-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/arm-cpu-features.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index eb8ddebffb..5444e4d40b 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -519,7 +519,6 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         bool kvm_supports_pmu;
         bool kvm_supports_steal_time;
         bool kvm_supports_sve;
-        char max_name[8], name[8];
         uint32_t max_vq, vq;
         uint64_t vls;
         QDict *resp;
@@ -573,9 +572,12 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         }
 
         if (kvm_supports_sve) {
+            g_autofree const char *max_name = NULL;
+            g_autofree const char *name = NULL;
+
             g_assert(vls != 0);
             max_vq = 64 - __builtin_clzll(vls);
-            sprintf(max_name, "sve%u", max_vq * 128);
+            max_name = g_strdup_printf("sve%u", max_vq * 128);
 
             /* Enabling a supported length is of course fine. */
             assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name);
@@ -583,6 +585,9 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
             /* Get the next supported length smaller than max-vq. */
             vq = 64 - __builtin_clzll(vls & ~BIT_ULL(max_vq - 1));
             if (vq) {
+                g_autofree const char *name2 =
+                    g_strdup_printf("sve%u", vq * 128);
+
                 /*
                  * We have at least one length smaller than max-vq,
                  * so we can disable max-vq.
@@ -595,11 +600,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
                  * unless all larger, supported vector lengths are also
                  * disabled.
                  */
-                sprintf(name, "sve%u", vq * 128);
-                error = g_strdup_printf("cannot disable %s", name);
+                error = g_strdup_printf("cannot disable %s", name2);
                 assert_error(qts, "host", error,
                              "{ %s: true, %s: false }",
-                             max_name, name);
+                             max_name, name2);
                 g_free(error);
             }
 
@@ -608,7 +612,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
              * we need at least one vector length enabled.
              */
             vq = __builtin_ffsll(vls);
-            sprintf(name, "sve%u", vq * 128);
+            name = g_strdup_printf(name, "sve%u", vq * 128);
             error = g_strdup_printf("cannot disable %s", name);
             assert_error(qts, "host", error, "{ %s: false }", name);
             g_free(error);
@@ -620,9 +624,11 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
                 }
             }
             if (vq <= SVE_MAX_VQ) {
-                sprintf(name, "sve%u", vq * 128);
-                error = g_strdup_printf("cannot enable %s", name);
-                assert_error(qts, "host", error, "{ %s: true }", name);
+                g_autofree const char *name2 =
+                    g_strdup_printf("sve%u", vq * 128);
+
+                error = g_strdup_printf("cannot enable %s", name2);
+                assert_error(qts, "host", error, "{ %s: true }", name2);
                 g_free(error);
             }
         } else {
-- 
2.43.0

Re: [PATCH 2/2] tests/qtest/arm-cpu-features: Use g_strdup_printf() instead of char arrays

[Philippe Mathieu-Daudé]

On 10/3/26 16:15, Peter Maydell wrote:
> Older versions of gcc with -Wformat-overflow=2 don't like the usage of
> fixed size char arrays in this test; gcc 7.5.0 (SUSE Linux) says:
> 
> ../tests/qtest/arm-cpu-features.c: In function ‘test_query_cpu_model_expansion_kvm’:
> ../tests/qtest/arm-cpu-features.c:578:35: error: ‘%u’ directive writing
> between 1 and 10 bytes into a region of size 5
> [-Werror=format-overflow=]
> 
> These can't actually happen because of the limited size of the values
> being substituted in.  However rather than require readers to check
> whether the arrays really have been declared large enough, we prefer
> to use g_strdup_printf() for this kind of string work.
> 
> Reported-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/arm-cpu-features.c | 24 +++++++++++++++---------
>   1 file changed, 15 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/2] tests/qtest: Avoid char arrays in some Arm tests

[Fabiano Rosas]

Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset fixes a couple of problems reported by Fabiano Rosas
> with gcc 7.5.0 now we have turned on -Wformat-overflow=2. In both
> cases the test is using sprintf() into a fixed-size char array, and
> it can't actually overflow but gcc doesn't realize that. We prefer
> to use g_strdup_printf() for this kind of thing anyway, as it is
> much less error-prone, so switch to that.
>
> thanks
> -- PMM
>
> Peter Maydell (2):
>   tests/qtest/ast2700-sgpio-test: Use g_strdup_printf() instead of char
>     arrays
>   tests/qtest/arm-cpu-features: Use g_strdup_printf() instead of char
>     arrays
>
>  tests/qtest/arm-cpu-features.c   | 24 +++++++++++++++---------
>  tests/qtest/ast2700-sgpio-test.c | 24 ++++++++++++------------
>  2 files changed, 27 insertions(+), 21 deletions(-)

Series:

Tested-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH 0/2] tests/qtest: Avoid char arrays in some Arm tests

[Cédric Le Goater]

On 3/10/26 16:15, Peter Maydell wrote:
> This patchset fixes a couple of problems reported by Fabiano Rosas
> with gcc 7.5.0 now we have turned on -Wformat-overflow=2. In both
> cases the test is using sprintf() into a fixed-size char array, and
> it can't actually overflow but gcc doesn't realize that. We prefer
> to use g_strdup_printf() for this kind of thing anyway, as it is
> much less error-prone, so switch to that.
> 
> thanks
> -- PMM
> 
> Peter Maydell (2):
>    tests/qtest/ast2700-sgpio-test: Use g_strdup_printf() instead of char
>      arrays
>    tests/qtest/arm-cpu-features: Use g_strdup_printf() instead of char
>      arrays
> 
>   tests/qtest/arm-cpu-features.c   | 24 +++++++++++++++---------
>   tests/qtest/ast2700-sgpio-test.c | 24 ++++++++++++------------
>   2 files changed, 27 insertions(+), 21 deletions(-)
> 

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.