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

[Stefan Weil <weil@mail.berlios.de>]

Am 26.06.2011 20:06, schrieb Blue Swirl:
> On Thu, Jun 23, 2011 at 6: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>
>>>>> --
...
>>>> 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:
>>
>> #include "config.h"        /* optional */
>> #include "qemu-common.h"
>> #include <system includes> /* without those that are included from
>> qemu-common.h */
>> ...
>> #include "other local includes"
>> ...
>>
>> As long as the maintainers don't accept patches which simply move
>> qemu-common.h, there is no danger. :-)
>>
>> Of course a comment might be added. In most cases, I'm a great friend
>> of good comments, but in this special case, I don't think it is 
>> necessary.
>> It's a very special case of a typedef conflict caused by the mingw32
>> version of jpeglib (which is obviously rarely used), and it might be 
>> fixed
>> in a newer version of jpeglib (so the comment would be no longer
>> valid, and nobody would notice that).
>
> We could also add a configure time check for this case for maximum
> over engineering.

... which nobody wants. I suggest to apply the patch as it was sent
by Roy. Stefan H. suggests to add a comment before the patch is applied.

Both ways fix a (small) problem, so please just decide which
solution you prefer.

Cheers,
Stefan W.