On 09/11/2020 13.50, Markus Armbruster wrote: > Alex Chen <alex.c...@huawei.com> writes: > >> On 2020/11/9 15:57, Markus Armbruster wrote: >>> Thomas Huth <th...@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 <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(-) >>>>>>>>>> >> [...] >>>>>>>>>> >>>>>>>>> >>>>>>>>> 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.
Just FYI, there are also apparently toolchains where uint32_t is defined as unsigned long: https://patchwork.kernel.org/project/kvm/patch/20201105135936.55088-1-alexandru.eli...@arm.com/ Thomas
