Re: [Qemu-devel] [PATCH v2 06/30] trace: Fix parameter types in migration

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]