On Mon, Jun 4, 2012 at 10:10 AM, Kai Tietz <ktiet...@googlemail.com> wrote:
> 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.
>

There is no such change in the patch but only member name changes
in the mentioned structure??  (fwiw, the member name changes seemed
correct to me, as well as the the len parameter type change from LONG to ULONG
for NtQueryInformationFile and the newly added FILE_LINK_INFORMATION.)

>
> Regards,
> Kai

--
O.S.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Reply via email to