On Tue, Apr 21, 2020 at 12:41 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Apr 21, 2020 at 12:51 PM Amit Kapila <amit.kapil...@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 *' > 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 . > >
0001-PG-compilation-error-with-VS-2015-2017-2019_v10.patch
Description: Binary data
