From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59979) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qavv9-0004Lw-P5 for qemu-devel@nongnu.org; Sun, 26 Jun 2011 16:27:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qavv8-0002GR-Fw for qemu-devel@nongnu.org; Sun, 26 Jun 2011 16:27:07 -0400 MIME-Version: 1.0 In-Reply-To: <4E07907F.1060703@mail.berlios.de> References: <4DFC636D.9090800@mail.berlios.de> <20110623135202.GA12765@stefanha-thinkpad.localdomain> <4E03564A.5060201@mail.berlios.de> <4E07907F.1060703@mail.berlios.de> From: Blue Swirl Date: Sun, 26 Jun 2011 23:26:45 +0300 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-trivial@nongnu.org, Stefan Hajnoczi , qemu-devel , Roy Tam On Sun, Jun 26, 2011 at 11:03 PM, Stefan Weil wrote: > Am 26.06.2011 20:06, schrieb Blue Swirl: >> >> On Thu, Jun 23, 2011 at 6:05 PM, Stefan Weil =C2= =A0wrote: >>> >>> 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 includi= ng >>>>>> qemu-common.h first. >>>>>> >>>>>> >>>>>> Sign-off-by: Roy Tam >>>>>> -- > > ... >>>>> >>>>> 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 tr= y >>>> to #define INT32? >>>> http://msdn.microsoft.com/en-us/library/aa383751(v=3Dvs.85).aspx >>>> >>>> In that case I think an explicit fix is better: >>>> >>>> #ifdef _WIN32 >>>> /* Include this before jpeglib.h for the INT32 definition */ >>>> #include >>>> #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" =C2=A0 =C2=A0 =C2=A0 =C2=A0/* optional */ >>> #include "qemu-common.h" >>> #include /* 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.