This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
Re: [Mingw-w64-public] Fwd: [PATCH v1] setup: allow building with i686-w64-mingw32
- From: Kai Tietz <ktietz70 at googlemail dot com>
- To: mingw-w64-public at lists dot sourceforge dot net
- Cc: cygwin-apps at cygwin dot com
- Date: Mon, 4 Jun 2012 09:10:31 +0200
- Subject: Re: [Mingw-w64-public] Fwd: [PATCH v1] setup: allow building with i686-w64-mingw32
- References: <4FCB7271.7040907@users.sourceforge.net> <4FCB86AE.8090007@users.sourceforge.net> <4FCC1D6F.7020202@users.sourceforge.net>
Hello,
here is my review about Yaakov's patch.
The change in crt/wchar.h about __mingw_ovr macro looks wrong to me,
or at least inconsistent. As stdio.h has same macro. At least a
libstdc++ bootstrap test is required for this change.
The hunks about guiddef.h are ok.
The hunk about ntdef.h is ok. Also the changes for propkeydef.h header, too.
The hunk in winnt.h:
@@ -88,7 +88,7 @@ extern "C" {
#include <basetsd.h>
-#if defined(_X86_) || defined(__ia64__) || defined(__x86_64)
+#if defined(__W32API_USE_DLLIMPORT__) && (defined(_X86_) ||
defined(__ia64__) || defined(__x86_64))
#define DECLSPEC_IMPORT __declspec(dllimport)
#else
#define DECLSPEC_IMPORT
I would like to see here a different macro-name and the logic needs to
be inverted. Our default scenario uses dllimport and just in cygwin's
setup-case we don't want it.
So I would like to see here instead: !defined (__NO_USE_DLLIMPORT)
Rest of the hunks for winnt.h are ok.
The hunks about winternl.header are ok, beside one nit. The change in
FILE_RENAME_INFORMATION about RootDir from HANDLE to ULONG type I am
not sure. If it should be a integer-scalar instead of a pointer, is
it sure that it is for 64-bit also just 32-bit wide? I would think we
should use here DWORD_PTR instead.
Regards,
Kai