On Tue, Apr 28, 2020 at 11:39 PM Juan José Santamaría Flecha
<juanjo.santama...@gmail.com> wrote:
>
>
> On Mon, Apr 27, 2020 at 1:20 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>>
>> I think we should backpatch this till 9.5 as I could see the changes
>> made by commit 0fb54de9 to support MSVC2015 are present in that branch
>> and the same is mentioned in the commit message.  Would you like to
>> prepare patches (and test those) for back-branches?
>
>
> I do not have means to test these patches using Visual Studio previous to 
> 2012, but please find attached patches for 9.5-9.6 and 10-11-12 as of version 
> 14. The extension is 'txt' not to break the cfbot.
>

I see some problems with these patches.
1.
+ loct = _create_locale(LC_CTYPE, winlocname);
+ if (loct != NULL)
+ {
+ lcid = loct->locinfo->lc_handle[LC_CTYPE];
+ if (lcid == 0)
+ lcid = MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), SORT_DEFAULT);
+ _free_locale(loct);
+ }

  if (!GetLocaleInfoA(lcid, LOCALE_SISO639LANGNAME, isolang, sizeof(isolang)))
  return NULL;
  if (!GetLocaleInfoA(lcid, LOCALE_SISO3166CTRYNAME, isocrty, sizeof(isocrty)))
  return NULL;

In the above change even if loct is NULL, we call GetLocaleInfoA()
which is wrong and the same is not a problem without the patch.

2. I think the code in IsoLocaleName is quite confusing and difficult
to understand in back branches and the changes due to this bug-fix
made it more complicated.  I am thinking to refactor it such that the
code for (_MSC_VER >= 1700 && _MSC_VER  < 1900), (_MSC_VER >= 1900)
and last #else code (the code for version < 17) resides in their own
functions.  That might make this function easier to understand, what
do you think?

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply via email to