[Mingw-w64-public] Fwd: [PATCH v1] setup: allow building with i686-w64-mingw32

Kai Tietz ktietz70@googlemail.com
Mon Jun 4 07:10:00 GMT 2012


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



More information about the Cygwin-apps mailing list