Thomas Huth <th...@redhat.com> writes: > 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.ro...@huawei.com> >>>> Signed-off-by: Alex Chen <alex.c...@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.
Yes, it works. > But if there is ever a compiler where the size of int is > different, you'll get a compiler warning here again. No, we won't. If we ever use a compiler where int is narrower than 32 bits, then the type of the argument is actually uint32_t[1]. We can forget about this case, because "int narrower than 32 bits" is not going to fly with our code base. If we ever use a compiler where int is wider than 32 bits, then the type of the argument is *not* uint32_t[2]. PRIu32 will work anyway, because it will actually retrieve an unsigned int argument, *not* an uint32_t argument[3]. In other words "%" PRIu32 is just a less legible alias for "%u" in all cases that matter. And that's why I would simply use "%u". > So if we now fix this > up, then let's do it really right and use PRIu32, please. > > Thomas [1] Because promotion does nothing either argument, and the usual arithmetic conversions convert to uint32_t. See my first reply. [2] Because uint32_t gets promoted to unsigned int. See my first reply. [3] Because variable arguments undergo default argument promotion (§ 6.5.2.2 Function calls), which promotes uint32_t to unsigned int.