* Re: [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified [not found] <BANLkTika6YmzRYAnta8TgQGpLSpoAdpKNQ@mail.gmail.com> @ 2011-06-18 8:35 ` Stefan Weil 2011-06-23 13:52 ` Stefan Hajnoczi 0 siblings, 1 reply; 9+ 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-18 8:35 ` [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified Stefan Weil @ 2011-06-23 13:52 ` Stefan Hajnoczi 2011-06-23 15:05 ` Stefan Weil 0 siblings, 1 reply; 9+ 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-23 13:52 ` 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; 9+ 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [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; 9+ 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [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; 9+ messages in thread From: Blue Swirl @ 2011-06-26 18:06 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [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; 9+ messages in thread From: Stefan Weil @ 2011-06-26 20:03 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-trivial, 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [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; 9+ messages in thread From: Blue Swirl @ 2011-06-26 20:26 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-trivial, 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [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 0 siblings, 1 reply; 9+ messages in thread From: TeLeMan @ 2011-06-27 2:37 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-trivial, Stefan Weil, Roy Tam, qemu-devel 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] 9+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified 2011-06-27 2:37 ` TeLeMan @ 2011-06-27 4:15 ` Roy Tam 0 siblings, 0 replies; 9+ messages in thread From: Roy Tam @ 2011-06-27 4:15 UTC (permalink / raw) To: TeLeMan; +Cc: Blue Swirl, Stefan Weil, qemu-trivial, qemu-devel 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] 9+ messages in thread
end of thread, other threads:[~2011-06-27 4:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <BANLkTika6YmzRYAnta8TgQGpLSpoAdpKNQ@mail.gmail.com>
2011-06-18 8:35 ` [Qemu-trivial] [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified Stefan Weil
2011-06-23 13:52 ` 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
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).