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

On Tue, Apr 21, 2020 at 12:41 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

> On Tue, Apr 21, 2020 at 12:51 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > I have tried a simple test with the latest patch and it failed for me.
> >
> > Set LC_MESSAGES="English_United Kingdom";
> > -- returns en-GB, then code changes it to en_NZ when _create_locale()
> > is used whereas with the patch it returns "" (empty string).
> >
> > There seem to be two problems here (a) The input to enum_locales_fn
> > doesn't seem to get the input name as "English_United Kingdom" due to
> > which it can't find a match even if the same exists. (b) After
> > executing EnumSystemLocalesEx, there is no way the patch can detect if
> > it is successful in finding the passed name due to which it appends
> > empty string in such cases.
>
> Few more comments:
> 1. I have tried the first one in the list provided by you and that
> also didn't work. Basically, I got empty string when I tried Set
> LC_MESSAGES='Afar';
>

I cannot reproduce any of these errors on my end. When using
_create_locale(), returning "en_NZ" is also a wrong result.

> 2. Getting below warning
> pg_locale.c(1072): warning C4133: 'function': incompatible types -
> from 'const char *' to 'const wchar_t *'
>

Yes, that is a regression.

> 3.
> + if (GetLocaleInfoEx(pStr, LOCALE_SENGLISHCOUNTRYNAME,
> + test_locale + len, LOCALE_NAME_MAX_LENGTH - len) > 0)
>
> All > or <= 0 checks should be changed to "!" types which mean to
> check whether the call toGetLocaleInfoEx is success or not.
>

MSVC does not recommend "!" in all cases, but GetLocaleInfoEx() looks fine,
so agreed.

4. In the patch, first, we try to get with LCType as LOCALE_SNAME and
> then with LOCALE_SENGLISHLANGUAGENAME and LOCALE_SENGLISHCOUNTRYNAME.
> I think we should add comments indicating why we try to get the locale
> information with three LCTypes and why the specific order of trying
> those types is required.
>

Agreed.

> 5. In one of the previous emails, you asked whether we have a list of
> supported locales. I don't find any such list. I think it depends on
> Windows locales for which you can get the information from
>
> https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-lcid/a9eac961-e77d-41a6-90a5-ce1a8b0cdb9c

Yes, that is the information we get from EnumSystemLocalesEx(), without the
additional entries _create_locale() has.

Please find attached a new version addressing the above mentioned, and so
adding a debug message for trying to get more information on the failed
cases.

Regards,

Juan José Santamaría Flecha
.

>
>