Patchwork fix MinGW compilation when --enable-vnc-jpeg is specified

login
register
mail settings
Submitter Roy Tam
Date June 18, 2011, 5:13 a.m.
Message ID <BANLkTika6YmzRYAnta8TgQGpLSpoAdpKNQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/100890/
State New
Headers show

Comments

Roy Tam - June 18, 2011, 5:13 a.m.
This patch fix conflicting types for 'INT32' in basetsd.h in including
qemu-common.h first.


Sign-off-by: Roy Tam  <roytam@gmail.com>
--
Stefan Weil - June 18, 2011, 8:35 a.m.
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.
Stefan Hajnoczi - June 23, 2011, 1:52 p.m.
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
Stefan Weil - June 23, 2011, 3:05 p.m.
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:

#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).

Stefan
Stefan Hajnoczi - June 24, 2011, 5:30 a.m.
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
Blue Swirl - June 26, 2011, 6:06 p.m.
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>
>>>> --
>>>> 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:
>
> #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.
Stefan Weil - June 26, 2011, 8:03 p.m.
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.
Blue Swirl - June 26, 2011, 8:26 p.m.
On Sun, Jun 26, 2011 at 11:03 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> 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.

I applied the patch with a comment, thanks.
TeLeMan - June 27, 2011, 2:37 a.m.
This patch breaks the compilation with --enable-vnc-png:

  CC    ui/vnc-enc-tight.o
In file included from /usr/include/png.h:518,
                 from ui/vnc-enc-tight.c:34:
/usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
'__attribute__' before '.' token
/usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'include'
make: *** [ui/vnc-enc-tight.o] Error 1

--
SUN OF A BEACH



On Mon, Jun 27, 2011 at 04:26, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sun, Jun 26, 2011 at 11:03 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>> 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.
>
> I applied the patch with a comment, thanks.
>
>
Roy Tam - June 27, 2011, 4:15 a.m.
Hi,

2011/6/27 TeLeMan <geleman@gmail.com>:
> This patch breaks the compilation with --enable-vnc-png:
>
>  CC    ui/vnc-enc-tight.o
> In file included from /usr/include/png.h:518,
>                 from ui/vnc-enc-tight.c:34:
> /usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before '.' token
> /usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before 'include'
> make: *** [ui/vnc-enc-tight.o] Error 1
>

Works for me here.
But for the configure script, I need modifying to pass instead of giving error.

-    vnc_png_libs="-lpng"
+    vnc_png_libs="-lpng -lz"


> --
> SUN OF A BEACH
>
>
>
> On Mon, Jun 27, 2011 at 04:26, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sun, Jun 26, 2011 at 11:03 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>>> 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.
>>
>> I applied the patch with a comment, thanks.
>>
>>
>
Stefan Weil - June 27, 2011, 5:40 a.m.
Am 27.06.2011 04:37, schrieb TeLeMan:
> This patch breaks the compilation with --enable-vnc-png:
>
> CC ui/vnc-enc-tight.o
> In file included from /usr/include/png.h:518,
> from ui/vnc-enc-tight.c:34:
> /usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before '.' token
> /usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before 'include'
> make: *** [ui/vnc-enc-tight.o] Error 1
>
> --
> SUN OF A BEACH

I get the same compiler error when I run with --enable-vnc-png,
and the buildbots got it as well. In my last test, png support
was disabled, so I did not notice this problem.

The png header files check for prior inclusion of setjmp.h
and have a really surprising way of telling that this happened
(instead of #error, they use illegal C code - a comment without
comment delimiters).

A patch which disables this strange check was just sent to qemu-devel.

Regards,
Stefan
Roy Tam - June 27, 2011, 5:56 a.m.
2011/6/27 Stefan Weil <weil@mail.berlios.de>:
> Am 27.06.2011 04:37, schrieb TeLeMan:
>>
>> This patch breaks the compilation with --enable-vnc-png:
>>
>> CC ui/vnc-enc-tight.o
>> In file included from /usr/include/png.h:518,
>> from ui/vnc-enc-tight.c:34:
>> /usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
>> '__attribute__' before '.' token
>> /usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
>> '__attribute__' before 'include'
>> make: *** [ui/vnc-enc-tight.o] Error 1
>>
>> --
>> SUN OF A BEACH
>
> I get the same compiler error when I run with --enable-vnc-png,
> and the buildbots got it as well. In my last test, png support
> was disabled, so I did not notice this problem.
>
> The png header files check for prior inclusion of setjmp.h
> and have a really surprising way of telling that this happened
> (instead of #error, they use illegal C code - a comment without
> comment delimiters).
>

I wonder which libpng version do you use. Mine is 1.5.2 and it works here.

> A patch which disables this strange check was just sent to qemu-devel.
>
> Regards,
> Stefan
>
>
Stefan Weil - June 27, 2011, 5:22 p.m.
Am 27.06.2011 07:56, schrieb Roy Tam:
> 2011/6/27 Stefan Weil<weil@mail.berlios.de>:
>> Am 27.06.2011 04:37, schrieb TeLeMan:
>>> This patch breaks the compilation with --enable-vnc-png:
>>>
>>> CC ui/vnc-enc-tight.o
>>> In file included from /usr/include/png.h:518,
>>> from ui/vnc-enc-tight.c:34:
>>> /usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
>>> '__attribute__' before '.' token
>>> /usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
>>> '__attribute__' before 'include'
>>> make: *** [ui/vnc-enc-tight.o] Error 1
>>>
>>> --
>>> SUN OF A BEACH
>> I get the same compiler error when I run with --enable-vnc-png,
>> and the buildbots got it as well. In my last test, png support
>> was disabled, so I did not notice this problem.
>>
>> The png header files check for prior inclusion of setjmp.h
>> and have a really surprising way of telling that this happened
>> (instead of #error, they use illegal C code - a comment without
>> comment delimiters).
>>
> I wonder which libpng version do you use. Mine is 1.5.2 and it works here.

The critical code is only active for linux (#ifdef __linux__ ... #endif).
I was using linbpng-1.2.42 (ubuntu) and libpng-1.2.44 (debian).

You won't get the problem when compiling with MinGW.
Paolo Bonzini - June 30, 2011, 3:26 p.m.
On 06/27/2011 07:22 PM, Stefan Weil wrote:
> Am 27.06.2011 07:56, schrieb Roy Tam:
>> 2011/6/27 Stefan Weil<weil@mail.berlios.de>:
>>> Am 27.06.2011 04:37, schrieb TeLeMan:
>>>> This patch breaks the compilation with --enable-vnc-png:
>>>>
>>>> CC ui/vnc-enc-tight.o
>>>> In file included from /usr/include/png.h:518,
>>>> from ui/vnc-enc-tight.c:34:
>>>> /usr/include/pngconf.h:371: error: expected '=', ',', ';', 'asm' or
>>>> '__attribute__' before '.' token
>>>> /usr/include/pngconf.h:372: error: expected '=', ',', ';', 'asm' or
>>>> '__attribute__' before 'include'
>>>> make: *** [ui/vnc-enc-tight.o] Error 1
>>>>
>>>> -- 
>>>> SUN OF A BEACH
>>> I get the same compiler error when I run with --enable-vnc-png,
>>> and the buildbots got it as well. In my last test, png support
>>> was disabled, so I did not notice this problem.
>>>
>>> The png header files check for prior inclusion of setjmp.h
>>> and have a really surprising way of telling that this happened
>>> (instead of #error, they use illegal C code - a comment without
>>> comment delimiters).
>>>
>> I wonder which libpng version do you use. Mine is 1.5.2 and it works 
>> here.
> 
> The critical code is only active for linux (#ifdef __linux__ ... #endif).
> I was using linbpng-1.2.42 (ubuntu) and libpng-1.2.44 (debian).
> 
> You won't get the problem when compiling with MinGW.

From pngconf.h

 * You can bypass this test if you know that your application uses exactly
 * the same setjmp.h that was included when libpng was built.  Only define
 * PNG_SKIP_SETJMP_CHECK while building your application, prior to the
 * application's '#include "png.h"'. Don't define PNG_SKIP_SETJMP_CHECK
 * while building a separate libpng library for general use.

Paolo

Patch

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"