| Message ID | BANLkTika6YmzRYAnta8TgQGpLSpoAdpKNQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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
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
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.
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.
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.
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.
>
>
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. >> >> >
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
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 > >
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.
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
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"