[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 06/30] trace: Fix parameter types in migratio
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 06/30] trace: Fix parameter types in migration |
Date: |
Wed, 15 Mar 2017 14:55:16 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 03/14/2017 09:36 AM, Eric Blake wrote:
>>>> So why is a PRIx64 not the right way to print a __u64 ?
>>>
>>> Because __u64 is not the same type as uint64_t. On 64-bit Linux, __u64
>>> is 'unsigned long long', while uint64_t is 'unsigned long'.
>>>
>>>> (I prefer %llx to the horrid PRIx64 syntax, but it still seems weird in
>>>> this case)
>>>
>>> As it is, I'm not sure if __u64 is always 'unsigned long long' in ALL
>>> Linux clients; an even-more-conservative patch would be to switch all
>>> callers to use explicit casts to something (like uint64_t or unsigned
>>> long long) that we have full control over, rather than passing __u64
>>> where we have no control over what type it ultimately resolves to.
>>
>> That would be my preference - casting to (uint64_t) at the caller and
>> keep this as PRIx64. We know it's a 64bit value so we should never
>> use unsigned long long anywhere for it.
>
> Okay, my next version will insert an explicit cast in any caller that is
> otherwise passing a __u64 (since we can't guarantee what type __u64
> resolves to, and therefore what format string is appropriate for it).
Or maybe I will just omit those changes from my next version, in lieu of
a gcc bug report. Here's my summary of an IRC conversation on the topic:
I just discovered that 32-bit mingw has a bug: it's <winsock.h> header
declared ntohl() as a function that returns 'u_long' ('unsigned long'),
but that POSIX requires ntohl() to return 'uin32_t' (which is 'unsigned
int' in that platform). The only workaround to buggy system headers is
to insert casts at all call sites, or to quit using overly-picky
-Wformat warnings.
The gist of my gcc bug report will be that newer gcc recently introduced
a fine-tuning for -Wformat, namely -Wformat-signedness. Some people
really do care about mismatches between "%d" and 'unsigned', or between
"%x" and 'int', but most of us don't. When -Wformat was originally
invented, there was a lot of 32-bit code, and very little 64-bit code,
so it made TOTAL sense that you wanted to flag mismatches between "%d"
and 'long', or between "%ld" and 'int', as it was a porting aid to
64-bit platforms. But these days, when we KNOW that a 32-bit platform
has 32-bit 'int' and 'long', and therefore that printf will behave
identically, it would be nice to squelch the warning, and instead only
issue it when there really is a mismatch (such as "%d" and 'long' on
64-bit). If there were such a knob (call it -Wformat-rank), then in
qemu, we'd turn it off (-Wno-format-rank) so we can play fast and sloppy
with same-width but different-rank typedefs (such as the kernel's __u64
vs. uint64_t, or mingw's 'u_long' vs. 'uint32_t), but still get the full
benefit of real warnings when there is a 32-bit vs. 64-bit difference.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 01/30] trace: Fix backwards mirror_yield parameters, (continued)
- [Qemu-devel] [PATCH v2 01/30] trace: Fix backwards mirror_yield parameters, Eric Blake, 2017/03/13
- [Qemu-devel] [PATCH v2 02/30] trace: Fix incorrect megasas trace parameters, Eric Blake, 2017/03/13
- [Qemu-devel] [PATCH v2 03/30] trace: Avoid abuse of amdvi_mmio_read, Eric Blake, 2017/03/13
- [Qemu-devel] [PATCH v2 05/30] trace: Fix parameter types in io, Eric Blake, 2017/03/13
- [Qemu-devel] [PATCH v2 06/30] trace: Fix parameter types in migration, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 04/30] trace: Fix parameter types in block, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 07/30] trace: Fix parameter types in ui, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 08/30] trace: Fix parameter types in top level, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 10/30] trace: Fix parameter types in hw/acpi, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 13/30] trace: Fix parameter types in hw/char, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 16/30] trace: Fix parameter types in hw/i386, Eric Blake, 2017/03/13
[Qemu-devel] [PATCH v2 11/30] trace: Fix parameter types in hw/audio, Eric Blake, 2017/03/13