* [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified @ 2011-06-18 5:13 Roy Tam 2011-06-18 8:35 ` Stefan Weil 0 siblings, 1 reply; 14+ messages in thread From: Roy Tam @ 2011-06-18 5:13 UTC (permalink / raw) To: qemu-devel 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" ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-18 5:13 [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified Roy Tam @ 2011-06-18 8:35 ` Stefan Weil 2011-06-23 13:52 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi 0 siblings, 1 reply; 14+ messages in thread From: Stefan Weil @ 2011-06-18 8:35 UTC (permalink / raw) To: Roy Tam; +Cc: qemu-trivial, qemu-devel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-18 8:35 ` Stefan Weil @ 2011-06-23 13:52 ` Stefan Hajnoczi 2011-06-23 15:05 ` Stefan Weil 0 siblings, 1 reply; 14+ messages in thread From: Stefan Hajnoczi @ 2011-06-23 13:52 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, qemu-devel, Roy Tam 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-23 13:52 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi @ 2011-06-23 15:05 ` Stefan Weil 2011-06-24 5:30 ` Stefan Hajnoczi 2011-06-26 18:06 ` Blue Swirl 0 siblings, 2 replies; 14+ messages in thread From: Stefan Weil @ 2011-06-23 15:05 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-trivial, qemu-devel, Roy Tam 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-23 15:05 ` Stefan Weil @ 2011-06-24 5:30 ` Stefan Hajnoczi 2011-06-26 18:06 ` Blue Swirl 1 sibling, 0 replies; 14+ messages in thread From: Stefan Hajnoczi @ 2011-06-24 5:30 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, qemu-devel, Roy Tam 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-23 15:05 ` Stefan Weil 2011-06-24 5:30 ` Stefan Hajnoczi @ 2011-06-26 18:06 ` Blue Swirl 2011-06-26 20:03 ` Stefan Weil 1 sibling, 1 reply; 14+ messages in thread From: Blue Swirl @ 2011-06-26 18:06 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, Stefan Hajnoczi, qemu-devel, Roy Tam 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-26 18:06 ` Blue Swirl @ 2011-06-26 20:03 ` Stefan Weil 2011-06-26 20:26 ` Blue Swirl 0 siblings, 1 reply; 14+ messages in thread From: Stefan Weil @ 2011-06-26 20:03 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-trivial, Stefan Hajnoczi, qemu-devel, Roy Tam 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-26 20:03 ` Stefan Weil @ 2011-06-26 20:26 ` Blue Swirl 2011-06-27 2:37 ` TeLeMan 0 siblings, 1 reply; 14+ messages in thread From: Blue Swirl @ 2011-06-26 20:26 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, Stefan Hajnoczi, qemu-devel, Roy Tam 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-26 20:26 ` Blue Swirl @ 2011-06-27 2:37 ` TeLeMan 2011-06-27 4:15 ` Roy Tam 2011-06-27 5:40 ` Stefan Weil 0 siblings, 2 replies; 14+ messages in thread From: TeLeMan @ 2011-06-27 2:37 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-trivial, Roy Tam, qemu-devel, Stefan Hajnoczi 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. > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 2:37 ` TeLeMan @ 2011-06-27 4:15 ` Roy Tam 2011-06-27 5:40 ` Stefan Weil 1 sibling, 0 replies; 14+ messages in thread From: Roy Tam @ 2011-06-27 4:15 UTC (permalink / raw) To: TeLeMan; +Cc: Blue Swirl, qemu-trivial, qemu-devel, Stefan Hajnoczi 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. >> >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 2:37 ` TeLeMan 2011-06-27 4:15 ` Roy Tam @ 2011-06-27 5:40 ` Stefan Weil 2011-06-27 5:56 ` Roy Tam 1 sibling, 1 reply; 14+ messages in thread From: Stefan Weil @ 2011-06-27 5:40 UTC (permalink / raw) To: TeLeMan; +Cc: Blue Swirl, Stefan Hajnoczi, qemu-devel, Roy Tam 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 5:40 ` Stefan Weil @ 2011-06-27 5:56 ` Roy Tam 2011-06-27 17:22 ` Stefan Weil 0 siblings, 1 reply; 14+ messages in thread From: Roy Tam @ 2011-06-27 5:56 UTC (permalink / raw) To: Stefan Weil; +Cc: Blue Swirl, TeLeMan, qemu-devel, Stefan Hajnoczi 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 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 5:56 ` Roy Tam @ 2011-06-27 17:22 ` Stefan Weil 2011-06-30 15:26 ` Paolo Bonzini 0 siblings, 1 reply; 14+ messages in thread From: Stefan Weil @ 2011-06-27 17:22 UTC (permalink / raw) To: Roy Tam; +Cc: qemu-devel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 17:22 ` Stefan Weil @ 2011-06-30 15:26 ` Paolo Bonzini 0 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2011-06-30 15:26 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-devel, Roy Tam 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-06-30 15:27 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-18 5:13 [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified Roy Tam 2011-06-18 8:35 ` Stefan Weil 2011-06-23 13:52 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi 2011-06-23 15:05 ` Stefan Weil 2011-06-24 5:30 ` Stefan Hajnoczi 2011-06-26 18:06 ` Blue Swirl 2011-06-26 20:03 ` Stefan Weil 2011-06-26 20:26 ` Blue Swirl 2011-06-27 2:37 ` TeLeMan 2011-06-27 4:15 ` Roy Tam 2011-06-27 5:40 ` Stefan Weil 2011-06-27 5:56 ` Roy Tam 2011-06-27 17:22 ` Stefan Weil 2011-06-30 15:26 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).