On Thu, Jun 23, 2011 at 6:05 PM, Stefan Weil <w...@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<roy...@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 <w...@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: > > #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.
