From: TeLeMan <geleman@gmail.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-trivial@nongnu.org, Roy Tam <roytam@gmail.com>,
qemu-devel <qemu-devel@nongnu.org>,
Stefan Hajnoczi <stefanha@gmail.com>
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified
Date: Mon, 27 Jun 2011 10:37:30 +0800 [thread overview]
Message-ID: <BANLkTikp8PZoZVbeuHVgDsH2mpLxKs+ytg@mail.gmail.com> (raw)
In-Reply-To: <BANLkTimZkQ9qBC+Rt0+CCV4TNKTRgttr9A@mail.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
--
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.
>
>
next prev parent reply other threads:[~2011-06-27 2:37 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BANLkTikp8PZoZVbeuHVgDsH2mpLxKs+ytg@mail.gmail.com \
--to=geleman@gmail.com \
--cc=blauwirbel@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-trivial@nongnu.org \
--cc=roytam@gmail.com \
--cc=stefanha@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).