* Michael Paquier wrote:

On Fri, Apr 8, 2016 at 10:05 PM, Andrew Dunstan <and...@dunslane.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).

--
Christian



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to