Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 16:57, Richard W.M. Jones wrote:
> On Fri, Mar 14, 2014 at 03:38:55PM +0000, Peter Maydell wrote:
>> On 14 March 2014 15:36, Richard W.M. Jones <address@hidden> wrote:
>>> On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
>>>> On 32-bit hosts, some compilers will warn on too large integer constants
>>>> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
>>>> on those defines.
>>>> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
>>>> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII 
>>>> */
>>>
>>> I think it's better to use this C99-defined feature (from <stdint.h>):
>>>
>>> #define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)
>>
>> Why? It's longer and we barely use it anywhere else
>> in the codebase, whereas we use the ULL suffix all
>> over the place...
> 
> It's permitted for unsigned long long to be longer than 64 bits.

Yes.

> The
> UINT64_C() macro will always return a 64 bit int constant.

That's incorrect, for two reasons:
(a) uint64_t (exact-width integer types in general) are optional in C99.
See 7.18.1.1 "Exact-width integer types" p3:

    These types are optional. However, if an implementation provides
    integer types with widths of 8, 16, 32, or 64 bits, it shall define
    the corresponding typedef names.

I general we can't state that the "UINT64_C() macro will always return a
64 bit int constant", because the C implementation might not even
support such a type.

(b) UINT64_C() is for "uint_least64_t" (7.18.4.1 Macros for
minimum-width integer constants). "uint_least64_t" is a required type
(7.18.1.2 Minimum-width integer types).

In practice I'd say it doesn't matter which one we use:
- ULL suffix is gnu89,
- UINT64_C() macro is gnu89,
- "unsigned long long" could be wider in general than 64 bits,
- "uint_least64_t" too could be wider in general than 64 bits,
- for us both results in uint64_t exactly.

So the above is a tie, but the ULL suffix is just nicer. (IMHO :))

Laszlo