Re: PG compilation error with Visual Studio 2015/2017/2019

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Juan José Santamaría Flecha <juanjo(dot)santamaria(at)gmail(dot)com>
Cc: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, davinder singh <davindersingh2692(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PG compilation error with Visual Studio 2015/2017/2019
Date: 2020-04-17 08:32:50
Message-ID: CAA4eK1LFXLUg5+2R2G+DoZ=D52WcELtzt3=4VMSqu6gA+ou61A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 15, 2020 at 11:58 PM Juan José Santamaría Flecha
<juanjo(dot)santamaria(at)gmail(dot)com> wrote:
>
> On Wed, Apr 15, 2020 at 4:46 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
>>
>> Em qua., 15 de abr. de 2020 às 03:08, davinder singh <davindersingh2692(at)gmail(dot)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

Attachment Content-Type Size
fix_create_locate_1.patch application/octet-stream 1.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2020-04-17 08:41:24 Re: Race condition in SyncRepGetSyncStandbysPriority
Previous Message Kyotaro Horiguchi 2020-04-17 08:07:06 Re: Race condition in SyncRepGetSyncStandbysPriority