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

[Alex Chen <alex.chen@huawei.com>]

On 2020/11/9 15:57, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 06/11/2020 15.18, Philippe Mathieu-Daudé wrote:
>>> On 11/6/20 7:33 AM, Markus Armbruster wrote:
>>>> Thomas Huth <thuth@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.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(-)
>>>>>>>>
[...]
>>>>>>>>
>>>>>>>
>>>>>>> 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.
>>
>> Agreed.
>>
>>>> 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].
>>
>> I can hardly believe that this can be true. Sure, it's true for such cases
>> like this one here, where you multiply with an "int". But if you just try to
>> print a plain uint32_t variable?
> 
> Default argument promotions (§6.5.2.2 Function calls) still apply: "the
> integer promotions are performed on each argument, and arguments that
> have type float are promoted to double."
> 
>> I've seen compiler warning in cases one tries to print a 16-bit (i.e. short)
>> variable in the past if you use %d instead of the proper PRId16 (or %hd)
>> format specifier - maybe not on x86, but certainly on other architectures.
>> If you're statement was right, that should not have happened, should it?
> 
> §7.19.6.1 "The fprintf function" on length modifier 'h':
> 
>     Specifies that a following d, i, o, u, x, or X conversion specifier
>     applies to a short int or unsigned short int argument (the argument
>     will have been promoted according to the integer promotions, but its
>     value shall be converted to short int or unsigned short int before
>     printing)
> 
> Integer promotions preserve value including sign.  So, printing a short
> value with %hd first promotes it to int, then converts it back to short.
> Neither conversion has an effect.
> 
> However, printing an int with %hd has: it converts int to short.
> Implementation-defined behavior when the value doesn't fit.
> 
> Length modifier 'h' is pretty pointless with printf().  So would be a
> warning to nudge people towards its use.
> 
> In fact, GNU libc's PRIu32 does not use it.  inttypes.h:
> 
>     /* Unsigned integers.  */
>     # define PRIu8		"u"
>     # define PRIu16		"u"
>     # define PRIu32		"u"
>     # define PRIu64		__PRI64_PREFIX "u"
> 
> where __PRI64_PREFIX is "l" or "ll" depending on system-dependent
> __WORDSIZE.
> 
> In short:
> 
>>>> In other words "%" PRIu32 is just a less legible alias for "%u" in all
>>>> cases that matter.
> 

Hi Markus,

Thanks for your reply, I have learned a lot.
May I understand it as follows:
%u is used when there are parameters obtained by arithmetic operation;
otherwise, PRIu32 is used to print uint32_t type parameters?

Thanks,
Alex