Re: [PATCH] qtest: Fix bad printf format specifiers

[Thomas Huth <thuth@redhat.com>]

On 05/11/2020 06.14, AlexChen wrote:
> On 2020/11/4 18:44, Thomas Huth wrote:
>> On 04/11/2020 11.23, AlexChen wrote:
>>> We should use printf format specifier "%u" instead of "%d" for
>>> argument of type "unsigned int".
>>>
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
>>> ---
>>>  tests/qtest/arm-cpu-features.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
>>> index d20094d5a7..bc681a95d5 100644
>>> --- a/tests/qtest/arm-cpu-features.c
>>> +++ b/tests/qtest/arm-cpu-features.c
>>> @@ -536,7 +536,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>>>          if (kvm_supports_sve) {
>>>              g_assert(vls != 0);
>>>              max_vq = 64 - __builtin_clzll(vls);
>>> -            sprintf(max_name, "sve%d", max_vq * 128);
>>> +            sprintf(max_name, "sve%u", max_vq * 128);
>>>
>>>              /* Enabling a supported length is of course fine. */
>>>              assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name);
>>> @@ -556,7 +556,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>>>                   * unless all larger, supported vector lengths are also
>>>                   * disabled.
>>>                   */
>>> -                sprintf(name, "sve%d", vq * 128);
>>> +                sprintf(name, "sve%u", vq * 128);
>>>                  error = g_strdup_printf("cannot disable %s", name);
>>>                  assert_error(qts, "host", error,
>>>                               "{ %s: true, %s: false }",
>>> @@ -569,7 +569,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%d", vq * 128);
>>> +            sprintf(name, "sve%u", vq * 128);
>>>              error = g_strdup_printf("cannot disable %s", name);
>>>              assert_error(qts, "host", error, "{ %s: false }", name);
>>>              g_free(error);
>>> @@ -581,7 +581,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>>>                  }
>>>              }
>>>              if (vq <= SVE_MAX_VQ) {
>>> -                sprintf(name, "sve%d", vq * 128);
>>> +                sprintf(name, "sve%u", vq * 128);
>>>                  error = g_strdup_printf("cannot enable %s", name);
>>>                  assert_error(qts, "host", error, "{ %s: true }", name);
>>>                  g_free(error);
>>>
>>
>> max_vq and vq are both "uint32_t" and not "unsigned int" ... so if you want
>> to fix this really really correctly, please use PRIu32 from inttypes.h instead.
>>
> 
> Hi Thomas,
> Thanks for your review.
> According to the definition of the macro PRIu32(# define PRIu32         "u"),
> using PRIu32 works the same as using %u to print, and using PRIu32 to print
> is relatively rare in QEMU(%u 720, PRIu32 only 120). Can we continue to use %u to
> print max_vq and vq in this patch.
> Of course, this is just my small small suggestion. If you think it is better to use
> PRIu32 for printing, I will send patch V2.

Well, %u happens to work since "int" is 32-bit with all current compilers
that we support. But if there is ever a compiler where the size of int is
different, you'll get a compiler warning here again. So if we now fix this
up, then let's do it really right and use PRIu32, please.

 Thomas