On Tue, Apr 21, 2020 at 12:51 PM Amit Kapila <amit.kapila16@gmail.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 *'
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.
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.
