PG compilation error with Visual Studio 2015/2017/2019

On Mon, Apr 6, 2020 at 1:08 PM davinder singh <davindersingh2692@gmail.com>
wrote:

I am working on “pg_locale compilation error with Visual Studio 2017”,
Related threads[1],[2].
We are getting compilation error in static char *IsoLocaleName(const char
*winlocname) function in pg_locale.c file. This function is trying to
convert the locale name into the Unix style. For example, it will change
the locale name "en-US" into "en_US".

How do you reproduce this issue with Visual Studio? I see there is an ifdef
directive above IsoLocaleName():

#if defined(WIN32) && defined(LC_MESSAGES)

I would expect defined(LC_MESSAGES) to be false in MSVC.

Regards,

Juan José Santamaría Flecha

On Mon, Apr 6, 2020 at 8:17 PM Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

How do you reproduce this issue with Visual Studio? I see there is an
ifdef directive above IsoLocaleName():

#if defined(WIN32) && defined(LC_MESSAGES)

I would expect defined(LC_MESSAGES) to be false in MSVC.

You need to enable NLS support in the config file. Let me know if that
answers your question.

--
Regards,
Davinder.

On Tue, Apr 7, 2020 at 7:44 AM davinder singh <davindersingh2692@gmail.com>
wrote:

You need to enable NLS support in the config file. Let me know if that
answers your question.

Yes, I can see where this is coming from now, thanks.

Currently, it is using _create_locale it behaves similarly to

GetLocaleInfoEx i.e. it also only checks the format only difference is for
a bigger set.
I thought of using GetLocaleInfoEx for the fix because it behaves
similarly to the already existing and a similar issue was resolved earlier
using the same. I have attached the patch, let me know your thoughts.

You are right, that the way to get the locale name information is through
GetLocaleInfoEx().

A couple of comments on the patch:

* I think you could save a couple of code lines, and make it clearer, by
merging both tests on  _MSC_VER  into a single #if... #else and leaving as
common code:
+ }
+ else
+ return NULL;
+#endif /*_MSC_VER && _MSC_VER < 1900*/

* The logic on "defined(_MSC_VER) && (_MSC_VER >= 1900)" is defined as
"_WIN32_WINNT >= 0x0600" on other parts of the code. I would
recommend using the later.

* This looks like a spurious change:
 - sizeof(iso_lc_messages), NULL);
+ sizeof(iso_lc_messages), NULL);

Regards,

Juan José Santamaría Flecha

On Tue, Apr 7, 2020 at 8:30 PM Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

* The logic on "defined(_MSC_VER) && (_MSC_VER >= 1900)" is defined as
"_WIN32_WINNT >= 0x0600" on other parts of the code. I would
recommend using the later.

I think "_WIN32_WINNT >= 0x0600" represents windows versions only and
doesn't include any information about Visual Studio versions. So I am
sticking to " defined(_MSC_VER) && (_MSC_VER >= 1900)".
I have resolved other comments. I have attached a new version of the patch.
--
Regards,
Davinder.

On Wed, Apr 8, 2020 at 9:03 AM davinder singh <davindersingh2692@gmail.com>
wrote:

On Tue, Apr 7, 2020 at 8:30 PM Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> wrote:

* The logic on "defined(_MSC_VER) && (_MSC_VER >= 1900)" is defined as
"_WIN32_WINNT >= 0x0600" on other parts of the code. I would
recommend using the later.

I think "_WIN32_WINNT >= 0x0600" represents windows versions only and
doesn't include any information about Visual Studio versions. So I am
sticking to " defined(_MSC_VER) && (_MSC_VER >= 1900)".

Let me explain further, in pg_config_os.h you can check that the value of
_WIN32_WINNT is solely based on checking _MSC_VER. This patch should also
be meaningful for WIN32 builds using MinGW, or we might see this issue
reappear in those systems if update the MIN_WINNT value to more current
OS versions. So, I still think _WIN32_WINNT is a better option.

I have resolved other comments. I have attached a new version of the patch.

I still see the same last lines in both #ifdef blocks, and pgindent might
change a couple of lines to:
+ MultiByteToWideChar(CP_ACP, 0, winlocname, -1, wc_locale_name,
+ LOCALE_NAME_MAX_LENGTH);
+
+ if ((GetLocaleInfoEx(wc_locale_name, LOCALE_SNAME,
+ (LPWSTR)&buffer, LOCALE_NAME_MAX_LENGTH)) > 0)
+ {

But that is pretty trivial stuff.

Please open an item in the commitfest for this patch.

Regards,

Juan José Santamaría Flecha

On Wed, Apr 8, 2020 at 7:39 PM Juan José Santamaría Flecha

Let me explain further, in pg_config_os.h you can check that the value of
_WIN32_WINNT is solely based on checking _MSC_VER. This patch should also
be meaningful for WIN32 builds using MinGW, or we might see this issue
reappear in those systems if update the MIN_WINNT value to more current
OS versions. So, I still think _WIN32_WINNT is a better option.

Thanks for explanation, I was not aware of that, you are right it make
sense to use " _WIN32_WINNT", Now I am using this only.

I still see the same last lines in both #ifdef blocks, and pgindent might

change a couple of lines to:
+ MultiByteToWideChar(CP_ACP, 0, winlocname, -1, wc_locale_name,
+ LOCALE_NAME_MAX_LENGTH);
+
+ if ((GetLocaleInfoEx(wc_locale_name, LOCALE_SNAME,
+ (LPWSTR)&buffer, LOCALE_NAME_MAX_LENGTH)) > 0)
+ {

Now I have resolved these comments also, Please check updated version of
the patch.

Please open an item in the commitfest for this patch.

I have created with same title.

--
Regards,
Davinder
EnterpriseDB: http://www.enterprisedb.com

On Wed, Apr 8, 2020 at 7:39 PM Juan José Santamaría Flecha

Let me explain further, in pg_config_os.h you can check that the value of
_WIN32_WINNT is solely based on checking _MSC_VER. This patch should also
be meaningful for WIN32 builds using MinGW, or we might see this issue
reappear in those systems if update the MIN_WINNT value to more current
OS versions. So, I still think _WIN32_WINNT is a better option.

Thanks for explanation, I was not aware of that, you are right it make
sense to use " _WIN32_WINNT", Now I am using this only.

I still see the same last lines in both #ifdef blocks, and pgindent might

change a couple of lines to:
+ MultiByteToWideChar(CP_ACP, 0, winlocname, -1, wc_locale_name,
+ LOCALE_NAME_MAX_LENGTH);
+
+ if ((GetLocaleInfoEx(wc_locale_name, LOCALE_SNAME,
+ (LPWSTR)&buffer, LOCALE_NAME_MAX_LENGTH)) > 0)
+ {

Now I have resolved these comments also, Please check updated version of
the patch.

Please open an item in the commitfest for this patch.

I have created with same title.

Hi,

I have a few comments about the patch, if I may.

1. Variable rc runs the risk of being used uninitialized.

2. Variable loct has a redundant declaration ( = NULL).

3. Return "C", does not solve the first case?

Attached, your patch with those considerations.

regards,

Ranier VIlela

On Thu, Apr 9, 2020 at 1:56 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

Attached, your patch with those considerations.

I see no attachment.

Regards

Em qui., 9 de abr. de 2020 às 09:14, Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> escreveu:

On Thu, Apr 9, 2020 at 1:56 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

Attached, your patch with those considerations.

I see no attachment.

Sorry, my mystake.

regards,
Ranier Vilela

On Mon, Apr 6, 2020 at 4:38 PM davinder singh
<davindersingh2692@gmail.com> wrote:

Hi All,

I am working on “pg_locale compilation error with Visual Studio 2017”, Related threads[1],[2].
We are getting compilation error in static char *IsoLocaleName(const char *winlocname) function in pg_locale.c file. This function is trying to convert the locale name into the Unix style. For example, it will change the locale name "en-US" into "en_US".
It is creating a locale using _locale_t _create_locale( int category, const char *locale) and then trying to access the name of that locale by pointing to internal elements of the structure loct->locinfo->locale_name[LC_CTYPE] but it has been missing from the _locale_t since VS2015 which is causing the compilation error. I found a few useful APIs that can be used here.

ResolveLocaleName and GetLocaleInfoEx both can take locale in the following format.
<language>-<REGION>
<language>-<Script>-<REGION>

ResolveLocaleName will try to find the closest matching locale name to the input locale name even if the input locale name is invalid given that the <language> is correct.

en-something-YZ => en-US
ex-US => error
Aa-aaaa-aa => aa-ET represents (Afar,Ethiopia)
Aa-aaa-aa => aa-ET

Refer [4] for more details

But in the case of GetLocaleInfoEx, it will only check the format of the input locale, as mentioned before, if correct it will return the name of the locale otherwise it will return an error.
For example.

en-something-YZ => error
ex-US => ex-US
aa-aaaa-aa => aa-Aaaa-AA
aa-aaa-aa => error.

Refer [5] for more details.

Currently, it is using _create_locale it behaves similarly to GetLocaleInfoEx i.e. it also only checks the format only difference is for a bigger set.
I thought of using GetLocaleInfoEx for the fix because it behaves similarly to the already existing and a similar issue was resolved earlier using the same.

It seems the right direction to use GetLocaleInfoEx as we have already
used it to handle a similar problem (lc_codepage is missing in
_locale_t in higher versions of MSVC (cf commit 0fb54de9aa)) in
chklocale.c. However, I see that we have added a manual parsing there
if GetLocaleInfoEx doesn't parse it. I think that addresses your
concern for _create_locale handling bigger sets. Don't we need
something equivalent here for the cases which GetLocaleInfoEx doesn't
support?

How have you ensured the testing of this code? I see that we have
src\test\locale in our test directory. Can we test using that?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Tue, Apr 7, 2020 at 8:30 PM Juan José Santamaría Flecha
<juanjo.santamaria@gmail.com> wrote:

* I think you could save a couple of code lines, and make it clearer, by merging both tests on  _MSC_VER  into a single #if... #else and leaving as common code:
+ }
+ else
+ return NULL;
+#endif /*_MSC_VER && _MSC_VER < 1900*/

* The logic on "defined(_MSC_VER) && (_MSC_VER >= 1900)" is defined as "_WIN32_WINNT >= 0x0600" on other parts of the code. I would recommend using the later.

I see that we have used _MSC_VER form of checks in win32_langinfo
(chklocale.c) for a similar kind of handling. So, isn't it better to
be consistent with that? Which exact part of the code you are
referring to?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Fri, Apr 10, 2020 at 5:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Apr 7, 2020 at 8:30 PM Juan José Santamaría Flecha
<juanjo.santamaria@gmail.com> wrote:

* I think you could save a couple of code lines, and make it clearer, by merging both tests on  _MSC_VER  into a single #if... #else and leaving as common code:
+ }
+ else
+ return NULL;
+#endif /*_MSC_VER && _MSC_VER < 1900*/

* The logic on "defined(_MSC_VER) && (_MSC_VER >= 1900)" is defined as "_WIN32_WINNT >= 0x0600" on other parts of the code. I would recommend using the later.

I see that we have used _MSC_VER form of checks in win32_langinfo
(chklocale.c) for a similar kind of handling. So, isn't it better to
be consistent with that? Which exact part of the code you are
referring to?

I see that the kind of check you are talking is recently added by
commit 352f6f2d. I think it is better to be consistent in all places.
Let's pick one and use that if possible.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Fri, Apr 10, 2020 at 2:25 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

I see that the kind of check you are talking is recently added by
commit 352f6f2d. I think it is better to be consistent in all places.
Let's pick one and use that if possible.

Currently there are two constructs to test the same logic, which is not
great. I think that using _MSC_VER makes it seem as MSVC exclusive code,
when MinGW should also be considered.

In the longterm aligning Postgres with MS product obsolescence will make
these tests unneeded, but I can propose a patch for making the test
consistent in all cases, on a different thread since this has little to do
with $SUBJECT.

Regards,

Juan José Santamaría Flecha

On Fri, Apr 10, 2020 at 5:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

It seems the right direction to use GetLocaleInfoEx as we have already
used it to handle a similar problem (lc_codepage is missing in
_locale_t in higher versions of MSVC (cf commit 0fb54de9aa)) in
chklocale.c. However, I see that we have added a manual parsing there
if GetLocaleInfoEx doesn't parse it. I think that addresses your
concern for _create_locale handling bigger sets. Don't we need
something equivalent here for the cases which GetLocaleInfoEx doesn't
support?

I am in investigating in similar lines, I think the following explanation
can help.
From Microsoft doc.
The locale argument to the setlocale, _wsetlocale, _create_locale, and
_wcreate_locale is
locale :: "locale-name"
| *"language[_country-region[.code-page]]"*
| ".code-page"
| "C"
| ""
| NULL

For GetLocaleInfoEx locale argument is
*<language>-<REGION>*
<language>-<Script>-<REGION>

As we can see _create_locale will also accept the locale appended with
code-page but that is not the case in lateral.
The important point is in our current issue we need locale name only and
both
functions(GetLocaleInfoEx, _create_locale) support an equal number of
locales
names if go by the syntax of the locale described on Microsoft documents.
With that thought, I am parsing the input
string to remove the code-page and give it as input to GelLocaleInfoEx.
I have attached the new patch.

How have you ensured the testing of this code? I see that we have
src\test\locale in our test directory. Can we test using that?

I don't know how to use these tests on windows, but I had a look in these
tests, I didn't found any test which could hit the function we are
modifying.
I m still working on testing this patch. If anyone has Idea please suggest.

[1]: https://docs.microsoft.com/en-us/windows/win32/intl/locale-names
[2]: https://docs.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getlocaleinfoex
https://docs.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getlocaleinfoex
[3]: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/create-locale-wcreate-locale?view=vs-2019 -- Regards, Davinder EnterpriseDB: http://www.enterprisedb.com
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/create-locale-wcreate-locale?view=vs-2019
--
Regards,
Davinder
EnterpriseDB: http://www.enterprisedb.com

Thanks for the review comments.

On Tue, Apr 14, 2020 at 9:12 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

I m still working on testing this patch. If anyone has Idea please

suggest.
I still see problems with this patch.

1. Variable loct have redundant initialization, it would be enough to
declare so: _locale_t loct;
2. Style white space in variable rc declaration.
3. Style variable cp_index can be reduced.
if (tmp != NULL) {
size_t cp_index;

cp_index = (size_t)(tmp - winlocname);
strncpy(loc_name, winlocname, cp_index);
loc_name[cp_index] = '\0';
4. Memory leak if _WIN32_WINNT >= 0x0600 is true, _free_locale(loct); is
not called.

I resolved the above comments.

5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is
not used?

_create_locale can take bigger input than GetLocaleInfoEx. But we are
interested in
*language[_country-region[.code-page]]*. We are using _create_locale to
validate
the given input. The reason is we can't verify the locale name if it is
appended with
code-page by using GetLocaleInfoEx. So before parsing, we verify if the
whole input
locale name is valid by using _create_locale. I hope that answers your
question.

I have attached the patch.
--
Regards,
Davinder
EnterpriseDB: http://www.enterprisedb.com

On Tue, Apr 14, 2020 at 1:07 PM davinder singh <davindersingh2692@gmail.com>
wrote:

On Fri, Apr 10, 2020 at 5:33 PM Amit Kapila <amit.kapila16@gmail.com>
wrote:

It seems the right direction to use GetLocaleInfoEx as we have already
used it to handle a similar problem (lc_codepage is missing in
_locale_t in higher versions of MSVC (cf commit 0fb54de9aa)) in
chklocale.c. However, I see that we have added a manual parsing there
if GetLocaleInfoEx doesn't parse it. I think that addresses your
concern for _create_locale handling bigger sets. Don't we need
something equivalent here for the cases which GetLocaleInfoEx doesn't
support?

I am in investigating in similar lines, I think the following explanation
can help.
From Microsoft doc.
The locale argument to the setlocale, _wsetlocale, _create_locale, and
_wcreate_locale is
locale :: "locale-name"
| *"language[_country-region[.code-page]]"*
| ".code-page"
| "C"
| ""
| NULL

For GetLocaleInfoEx locale argument is
*<language>-<REGION>*
<language>-<Script>-<REGION>

As we can see _create_locale will also accept the locale appended with
code-page but that is not the case in lateral.
The important point is in our current issue we need locale name only and
both
functions(GetLocaleInfoEx, _create_locale) support an equal number of
locales
names if go by the syntax of the locale described on Microsoft documents.
With that thought, I am parsing the input
string to remove the code-page and give it as input to GelLocaleInfoEx.
I have attached the new patch.

How have you ensured the testing of this code? I see that we have
src\test\locale in our test directory. Can we test using that?

I don't know how to use these tests on windows, but I had a look in these
tests, I didn't found any test which could hit the function we are
modifying.
I m still working on testing this patch. If anyone has Idea please suggest.

[1] https://docs.microsoft.com/en-us/windows/win32/intl/locale-names
[2]
https://docs.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getlocaleinfoex
[3]
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/create-locale-wcreate-locale?view=vs-2019
--
Regards,
Davinder
EnterpriseDB: http://www.enterprisedb.com

--
Regards,
Davinder
EnterpriseDB: http://www.enterprisedb.com

Em qua., 15 de abr. de 2020 às 03:08, davinder singh <
davindersingh2692@gmail.com> escreveu:

Thanks for the review comments.

On Tue, Apr 14, 2020 at 9:12 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

I m still working on testing this patch. If anyone has Idea please

suggest.
I still see problems with this patch.

1. Variable loct have redundant initialization, it would be enough to
declare so: _locale_t loct;
2. Style white space in variable rc declaration.
3. Style variable cp_index can be reduced.
if (tmp != NULL) {
size_t cp_index;

cp_index = (size_t)(tmp - winlocname);
strncpy(loc_name, winlocname, cp_index);
loc_name[cp_index] = '\0';
4. Memory leak if _WIN32_WINNT >= 0x0600 is true, _free_locale(loct); is
not called.

I resolved the above comments.

Ok, thanks.

5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is
not used?

_create_locale can take bigger input than GetLocaleInfoEx. But we are
interested in
*language[_country-region[.code-page]]*. We are using _create_locale to
validate
the given input. The reason is we can't verify the locale name if it is
appended with
code-page by using GetLocaleInfoEx. So before parsing, we verify if the
whole input
locale name is valid by using _create_locale. I hope that answers your
question.

Understood. In this case, _create_locale, is being used only to validate
the input.
Perhaps, in addition, you could create an additional function, which only
validates winlocname, without having to create structures or use malloc, to
be used when _WIN32_WINNT> = 0x0600 is true, but it is only a suggestion,
if you think it is necessary.

But have a last problem, in case GetLocaleInfoEx fail, there is still one
memory leak,
before return NULL is needed call: _free_locale(loct);

Another suggestion, if GetLocaleInfoEx fail wouldn't it be good to log the
error and the error number?

regards,
Ranier Vilela

On Wed, Apr 15, 2020 at 4:46 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

Em qua., 15 de abr. de 2020 às 03:08, davinder singh <
davindersingh2692@gmail.com> escreveu:

5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is

not used?

_create_locale can take bigger input than GetLocaleInfoEx. But we are
interested in
*language[_country-region[.code-page]]*. We are using _create_locale to
validate
the given input. The reason is we can't verify the locale name if it is
appended with
code-page by using GetLocaleInfoEx. So before parsing, we verify if the
whole input
locale name is valid by using _create_locale. I hope that answers your
question.

Understood. In this case, _create_locale, is being used only to validate
the input.
Perhaps, in addition, you could create an additional function, which only
validates winlocname, without having to create structures or use malloc, to
be used when _WIN32_WINNT> = 0x0600 is true, but it is only a suggestion,
if you think it is necessary.

Looking at the comments for IsoLocaleName() I see: "MinGW headers declare
_create_locale(), but msvcrt.dll lacks that symbol". This is outdated
[1]: https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/?limit=250&amp;page=2
should use GetLocaleInfoEx() as a complete alternative to _create_locale().

Please find attached a patch for so.

[1]: https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/?limit=250&amp;page=2
https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/?limit=250&amp;page=2
[2]: https://github.com/mirror/mingw-w64/commit/b508bb87ad

Regards,

Juan José Santamaría Flecha

Em qua., 15 de abr. de 2020 às 15:28, Juan José Santamaría Flecha <
juanjo.santamaria@gmail.com> escreveu:

On Wed, Apr 15, 2020 at 4:46 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

Em qua., 15 de abr. de 2020 às 03:08, davinder singh <
davindersingh2692@gmail.com> escreveu:

5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is

not used?

_create_locale can take bigger input than GetLocaleInfoEx. But we are
interested in
*language[_country-region[.code-page]]*. We are using _create_locale to
validate
the given input. The reason is we can't verify the locale name if it is
appended with
code-page by using GetLocaleInfoEx. So before parsing, we verify if the
whole input
locale name is valid by using _create_locale. I hope that answers your
question.

Understood. In this case, _create_locale, is being used only to validate
the input.
Perhaps, in addition, you could create an additional function, which only
validates winlocname, without having to create structures or use malloc, to
be used when _WIN32_WINNT> = 0x0600 is true, but it is only a suggestion,
if you think it is necessary.

Looking at the comments for IsoLocaleName() I see: "MinGW headers declare
_create_locale(), but msvcrt.dll lacks that symbol". This is outdated
[1][2], and _create_locale() could be used from Windows 8, but I think we
should use GetLocaleInfoEx() as a complete alternative to _create_locale().

Sounds good to me, the exception maybe log error in case fail?

regards,
Ranier Vilela

On Wed, Apr 15, 2020 at 11:58 PM Juan José Santamaría Flecha
<juanjo.santamaria@gmail.com> wrote:

On Wed, Apr 15, 2020 at 4:46 PM Ranier Vilela <ranier.vf@gmail.com> wrote:

Em qua., 15 de abr. de 2020 às 03:08, davinder singh <davindersingh2692@gmail.com> escreveu:

5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is not used?

_create_locale can take bigger input than GetLocaleInfoEx. But we are interested in
language[_country-region[.code-page]]. We are using _create_locale to validate
the given input. The reason is we can't verify the locale name if it is appended with
code-page by using GetLocaleInfoEx. So before parsing, we verify if the whole input
locale name is valid by using _create_locale. I hope that answers your question.

Understood. In this case, _create_locale, is being used only to validate the input.
Perhaps, in addition, you could create an additional function, which only validates winlocname, without having to create structures or use malloc, to be used when _WIN32_WINNT> = 0x0600 is true, but it is only a suggestion, if you think it is necessary.

Looking at the comments for IsoLocaleName() I see: "MinGW headers declare _create_locale(), but msvcrt.dll lacks that symbol". This is outdated [1][2], and _create_locale() could be used from Windows 8, but I think we should use GetLocaleInfoEx() as a complete alternative to _create_locale().

I see some differences in the output when _create_locale() is used vs.
when GetLocaleInfoEx() is used. Forex.

Set LC_MESSAGES="English_New Zealand";

-- returns en-NZ, then code changes it to en_NZ when _create_locale()
is used whereas GetLocaleInfoEx returns error.

Set LC_MESSAGES="English_Republic of the Philippines";

-- returns en-PH, then code changes it to en_PH when _create_locale()
is used whereas GetLocaleInfoEx returns error.

Set LC_MESSAGES="English_New Zealand";

-- returns en-NZ, then code changes it to en_NZ when _create_locale()
is used whereas GetLocaleInfoEx returns error.

Set LC_MESSAGES="French_Canada";

--returns fr-CA when _create_locale() is used whereas GetLocaleInfoEx
returns error.

The function IsoLocaleName() header comment says "Convert a Windows
setlocale() argument to a Unix-style one", so I was expecting above
cases which gives valid values with _create_locale() should also work
with GetLocaleInfoEx(). If it is fine for GetLocaleInfoEx() to return
an error for the above cases, then we need an explanation of the same
and probably add a few comments as well. So, I am not sure if we can
conclude that GetLocaleInfoEx() is an alternative to _create_locale()
at this stage.

I have used the attached hack to make _create_locale work on the
latest MSVC. Just to be clear this is mainly for the purpose of
testing the behavior _create_locale.

On the code side,
+ GetLocaleInfoEx(wc_locale_name, LOCALE_SNAME, (LPWSTR) &buffer,
+ LOCALE_NAME_MAX_LENGTH);

  /* Locale names use only ASCII, any conversion locale suffices. */
- rc = wchar2char(iso_lc_messages, loct->locinfo->locale_name[LC_CTYPE],
- sizeof(iso_lc_messages), NULL);
+ rc = wchar2char(iso_lc_messages, buffer, sizeof(iso_lc_messages), NULL);

Check the return value of GetLocaleInfoEx and if it is successful,
then only use wchar2char, otherwise, this function will return an
empty string ("") instead of NULL.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com