[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qtest: Fix bad printf format specifiers
From: |
Markus Armbruster |
Subject: |
Re: [PATCH] qtest: Fix bad printf format specifiers |
Date: |
Mon, 09 Nov 2020 13:50:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Alex Chen <alex.chen@huawei.com> writes:
> 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?
No. Use "%u" unless you need portability to machines where unsigned is
narrower than 32 bits (we don't).
On machines where unsigned int is at least 32 bit wide, "%" PRIu32
is the same as "%u". It's not wrong, just illegible.
- [PATCH] qtest: Fix bad printf format specifiers, AlexChen, 2020/11/04
- Re: [PATCH] qtest: Fix bad printf format specifiers, Thomas Huth, 2020/11/04
- Re: [PATCH] qtest: Fix bad printf format specifiers, AlexChen, 2020/11/05
- Re: [PATCH] qtest: Fix bad printf format specifiers, Thomas Huth, 2020/11/05
- Re: [PATCH] qtest: Fix bad printf format specifiers, Markus Armbruster, 2020/11/06
- Re: [PATCH] qtest: Fix bad printf format specifiers, Philippe Mathieu-Daudé, 2020/11/06
- Re: [PATCH] qtest: Fix bad printf format specifiers, Markus Armbruster, 2020/11/06
- Re: [PATCH] qtest: Fix bad printf format specifiers, Thomas Huth, 2020/11/08
- Re: [PATCH] qtest: Fix bad printf format specifiers, Markus Armbruster, 2020/11/09
- Re: [PATCH] qtest: Fix bad printf format specifiers, Alex Chen, 2020/11/09
- Re: [PATCH] qtest: Fix bad printf format specifiers,
Markus Armbruster <=
- Re: [PATCH] qtest: Fix bad printf format specifiers, Thomas Huth, 2020/11/10
- Re: [PATCH] qtest: Fix bad printf format specifiers, Markus Armbruster, 2020/11/11
Re: [PATCH] qtest: Fix bad printf format specifiers, Markus Armbruster, 2020/11/05