Re: [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified

[Stefan Hajnoczi <stefanha@gmail.com>]

On Thu, Jun 23, 2011 at 4:05 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 23.06.2011 15:52, schrieb Stefan Hajnoczi:
>>
>> On Sat, Jun 18, 2011 at 10:35:57AM +0200, Stefan Weil wrote:
>>>
>>> Am 18.06.2011 07:13, schrieb Roy Tam:
>>>>
>>>> This patch fix conflicting types for 'INT32' in basetsd.h in including
>>>> qemu-common.h first.
>>>>
>>>>
>>>> Sign-off-by: Roy Tam<roytam@gmail.com>
>>>> --
>>>> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
>>>> index 87fdf35..1591df0 100644
>>>> --- a/ui/vnc-enc-tight.c
>>>> +++ b/ui/vnc-enc-tight.c
>>>> @@ -28,6 +28,8 @@
>>>>
>>>> #include "config-host.h"
>>>>
>>>> +#include "qemu-common.h"
>>>> +
>>>> #ifdef CONFIG_VNC_PNG
>>>> #include<png.h>
>>>> #endif
>>>> @@ -36,8 +38,6 @@
>>>> #include<jpeglib.h>
>>>> #endif
>>>>
>>>> -#include "qemu-common.h"
>>>> -
>>>> #include "bswap.h"
>>>> #include "qint.h"
>>>> #include "vnc.h"
>>>
>>> Acked-by: Stefan Weil <weil@mail.berlios.de>
>>>
>>> The conflicting declaration is in jmorecfg.h which is included from
>>> jpeglib.h.
>>
>> Is the problem that the Windows headers included from qemu-common.h try
>> to #define INT32?
>> http://msdn.microsoft.com/en-us/library/aa383751(v=vs.85).aspx
>>
>> In that case I think an explicit fix is better:
>>
>> #ifdef _WIN32
>> /* Include this before jpeglib.h for the INT32 definition */
>> #include <basetsd.h>
>> #endif
>>
>> ...followed by png/jpeg includes...
>>
>> Simply moving qemu-common.h provides no hints and is rather indirect.
>> Someone may move it back in the future.
>>
>> Stefan
>
> INT32 is declared in basetsd.h which is included from windows.h
> (with some indirections) which is included from qemu-os-win32.h
> which is included from qemu-common.h.
>
> INT32 is not a #define, but a data type (typedef) and very common
> for w32 compilations. Windows programmers don't include basetsd.h
> directly, but usually use windows.h.
>
> Including qemu-common.h right at the beginning (after config.h where
> needed) should be good practice for QEMU source code - like this:

Too bad qemu-common.h doesn't just include system headers, it also
declares a bunch of QEMU stuff.

> As long as the maintainers don't accept patches which simply move
> qemu-common.h, there is no danger. :-)

Sure but isn't always noticed.  That is why I suggest adding a comment
about the jpeg interaction.

Stefan