Re: PG compilation error with Visual Studio 2015/2017/2019 - Mailing list pgsql-hackers

On Tue, Apr 21, 2020 at 5:32 PM Juan José Santamaría Flecha
<juanjo.santamaria@gmail.com> wrote:
>
> I cannot reproduce any of these errors on my end.
>
The first problem related to the English_United Kingdom was due to the
usage of wcslen instead of pg_mbstrlen to compute the length of
winlocname.  So, this is fixed with your latest patch.  I have
debugged the case for 'Afar' and found that _create_locale also didn't
return anything for that in my machine, so probably that locale
information is not there in my environment.

>  When using _create_locale(),  returning "en_NZ" is also a wrong result.

Hmm, that was a typo, it should be en_GB instead.

>> 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.
>

But, I don't see much in the comments?

Few more comments:
1.
  if (rc == -1 || rc == sizeof(iso_lc_messages))
- return NULL;
+
iso_lc_messages[0] = '\0';

I don't think this change is required.  The caller expects NULL in
case the API is not successful so that it can point result directly to
the locale passed.  I have changed this back to the original code in
the attached patch.

2.
I see some differences in the output of GetLocaleInfoEx and
_create_locale for some locales as mentioned in one of the documents
shared by you. Ex.

Bemba_Zambia bem-ZM bem
Bena_Tanzania bez-TZ bez
Bulgarian_Bulgaria bg-BG bg

Now, these might be okay but I think unless we test such things by
seeing the error message changes due to these locales we can't be
sure.

3. In the attached patch, I have handled one of the problem reported
earlier aka "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."

4. I think for the matter of this API, it is better to use _MSC_VER
related checks instead of _WIN32_WINNT so as to be consistent with
similar usage in chklocale.c (see win32_langinfo).  We can later
change the checks at all places to _WIN32_WINNT if required.  I have
changed this as well in the attached patch.

5. I am slightly nervous about the usage of wchar functions like
_wcsicmp, wcslen, etc. as those are not used anywhere in the code.
OTOH, I don't see any problem with that.  There is pg_wchar datatype
in the code and some corresponding functions to manipulate it.  Have
you considered using it?