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

[Blue Swirl]

On Sun, Jun 26, 2011 at 11:03 PM, Stefan Weil <address@hidden> wrote:
> Am 26.06.2011 20:06, schrieb Blue Swirl:
>>
>> On Thu, Jun 23, 2011 at 6:05 PM, Stefan Weil<address@hidden>  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<address@hidden>
>>>>>> --
>
> ...
>>>>>
>>>>> 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.

I applied the patch with a comment, thanks.