Re: VS 2015 support in src/tools/msvc

On 04/08/2016 11:02 AM, Christian Ullrich wrote:
> * Michael Paquier wrote:
>
>> On Fri, Apr 8, 2016 at 10:05 PM, Andrew Dunstan <andrew(at)dunslane(dot)net>
>> wrote:
>> ¥> On 04/08/2016 07:15 AM, Christian Ullrich wrote:
>
>>>> Michael, none of your patches change this, so how does it ever
>>>> build on
>>>> your system?
>>
>> Luck. I am getting a warning but the code is able to somewhat compile:
>> src/port/chklocale.c(230): warning C4013: 'GetLocaleInfoEx'
>> undefined; assuming extern returning int
>> [C:\Users\IEUser\git\postgres\libpgport.vcxproj]
>
> Weird. This assumed declaration is __cdecl, the actual function is
> __stdcall, and I think this should be guaranteed to crash.
>
>> But that's clearly incorrect to get that. As you are saying, what we
>> actually just need to do is bumping _WIN32_WINNT to 0x0600 when
>> compiling with VS2015, meaning that the minimum build requirement for
>> Postgres with VS2015 would be Windows Vista, and it would not be
>> possible to compile it on XP or Windows server 2k3. As XP is already
>> out of support, I think that this is an acceptable tradeoff, and it
>> would still be possible to build Postgres on XP with older versions of
>> Visual. Thoughts?
>
> I think you confuse two things here, let's call them "build
> environment" and "build platform". The build environment is whichever
> Windows SDK (among other things) is installed; if it is a version for
> Vista or later, that just means it has the declaration in the first
> place, and has the import in kernel32.lib. The build platform is the
> OS the compiler is run on; as long as you find a compiler that
> understands the headers from your chosen SDK version, you can run it
> on Windows 95 if both of you want.
>
> Changing _WIN32_WINNT also affects, indirectly, on which platforms the
> resulting binaries can run. Assume a macro that has an alternative
> definition, conditioned on _WIN32_WINNT >= _WIN32_WINNT_VISTA, that
> uses a function added in Vista. A binary built using this declaration,
> no matter where and when, will not run on anything older.
>
>> Anyway, attached are updated patches. This makes the warning go away
>> from my side, so I guess that it should allow Andrew to compile the
>> code.
>
> Which brings us to the next problem:
>
> src/port/chklocale.c(233): warning C4133: 'function': incompatible
> types - from 'const char *' to 'LPCWSTR' [...\postgres.vcxproj]
>
> I have no idea why I get this warning; I would have expected something
> more like this:
>
> localetest.cpp(26): error C2664: 'int
> GetLocaleInfoEx(LPCWSTR,LCTYPE,LPWSTR,int)': cannot convert
> argument 1 from 'const char *' to 'LPCWSTR'
>
> Apparently the warning is triggered by type mismatches in pointer
> arithmetic, although I can't see any here. Anyway, it concerns the
> first argument in this call to GetLocaleInfoEx(), which here is a
> const char*.
>
> According to the documentation and the prototype, however, it should
> be an LPCWSTR, because this function is Unicode-only (has no A/W
> variants). Unless LOCALE_IDEFAULTANSICODEPAGE also changes the
> interpretation of this first argument to a single-byte string, which
> is not mentioned anywhere in the documentation and makes no sense to
> begin with, I don't think this has ever worked either.
>
> I just tested it, and, of course, if I pass '(LPCWSTR)"de-DE"' (narrow
> string cast to LPCWSTR), the call fails with ERROR_INVALID_PARAMETER.
> With a wide string, I get the correct code page for the locale.
>
>
> Also, while you're at it, could you improve the comments a bit? I have
> not yet tried following the code to see which locale formats it uses
> where ("German_Germany", "de-DE", etc.), but GetLocaleInfoEx() takes
> the short form and there is a comment about the long form right below
> that call once patched (in the old code that gets turned into an else
> branch).
>

OK, well, we're making progress. I can confirm that using _WIN32_WINNT =
0x0600 fixes my problems - I can build and run the regression tests. I'm
inclined to define _WINSOCK_DEPRECATED_NO_WARNINGS to silence a few
compiler bleatings.

Do you have a fix for the LPCWSTR parameter issue?

cheers

andrew