[PATCH] msvcrt: Duplicate the secondary structs in _get_current_locale().

[Chip Davis]

Otherwise, the returned _locale_t will reflect the state after a
setlocale() call, and not before.

Respect the refcount pointers now; this is necessary to share malloc'd
stuff between _locale_t instances.

Signed-off-by: Chip Davis <cdavis(a)codeweavers.com>
---
 dlls/msvcr90/tests/msvcr90.c |  27 +++++++
 dlls/msvcrt/locale.c         | 146 ++++++++++++++++++++++++++---------
 2 files changed, 137 insertions(+), 36 deletions(-)

diff --git a/dlls/msvcr90/tests/msvcr90.c b/dlls/msvcr90/tests/msvcr90.c
index c8a49f25492d..07403c6b2fc7 100644
--- a/dlls/msvcr90/tests/msvcr90.c
+++ b/dlls/msvcr90/tests/msvcr90.c
@@ -139,6 +139,7 @@ static int (__cdecl *p_swscanf)(const wchar_t *str, const wchar_t* format, ...);
 static int (__cdecl *p____mb_cur_max_l_func)(_locale_t locale);
 static _locale_t (__cdecl *p__create_locale)(int, const char*);
 static void (__cdecl *p__free_locale)(_locale_t);
+static _locale_t (__cdecl *p__get_current_locale)(void);
 
 /* make sure we use the correct errno */
 #undef errno
@@ -408,6 +409,7 @@ static BOOL init(void)
     SET(p____mb_cur_max_l_func, "___mb_cur_max_l_func");
     SET(p__create_locale, "_create_locale");
     SET(p__free_locale, "_free_locale");
+    SET(p__get_current_locale, "_get_current_locale");
 
     if (sizeof(void *) == 8)
     {
@@ -1961,6 +1963,30 @@ static void test____mb_cur_max_l_func(void)
     p__free_locale(l);
 }
 
+static void test__get_current_locale(void)
+{
+    _locale_t l = p__get_current_locale();
+#define lc_str(lc, s) \
+    ok(!strcmp(l->locinfo->lc_category[lc].locale, s), #lc " = \"%s\"\n", \
+        l->locinfo->lc_category[lc].locale)
+    lc_str(LC_COLLATE, "C");
+    lc_str(LC_CTYPE, "C");
+    lc_str(LC_MONETARY, "C");
+    lc_str(LC_NUMERIC, "C");
+    lc_str(LC_TIME, "C");
+
+    p_setlocale(LC_ALL, "english");
+    lc_str(LC_COLLATE, "C");
+    lc_str(LC_CTYPE, "C");
+    lc_str(LC_MONETARY, "C");
+    lc_str(LC_NUMERIC, "C");
+    lc_str(LC_TIME, "C");
+#undef lc_str
+
+    p__free_locale(l);
+    p_setlocale(LC_ALL, "C");
+}
+    
 START_TEST(msvcr90)
 {
     if(!init())
@@ -2001,4 +2027,5 @@ START_TEST(msvcr90)
     test___strncnt();
     test_swscanf();
     test____mb_cur_max_l_func();
+    test__get_current_locale();
 }
diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 04a808d6b9c4..21e155d452c1 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -876,6 +876,9 @@ void free_locinfo(MSVCRT_pthreadlocinfo locinfo)
         return;
 
     for(i=MSVCRT_LC_MIN+1; i<=MSVCRT_LC_MAX; i++) {
+        if(!locinfo->lc_category[i].refcount ||
+            InterlockedDecrement(locinfo->lc_category[i].refcount))
+            continue;
         MSVCRT_free(locinfo->lc_category[i].locale);
         MSVCRT_free(locinfo->lc_category[i].refcount);
 #if _MSVCR_VER >= 110
@@ -884,37 +887,51 @@ void free_locinfo(MSVCRT_pthreadlocinfo locinfo)
     }
 
     if(locinfo->lconv) {
-        MSVCRT_free(locinfo->lconv->decimal_point);
-        MSVCRT_free(locinfo->lconv->thousands_sep);
-        MSVCRT_free(locinfo->lconv->grouping);
-        MSVCRT_free(locinfo->lconv->int_curr_symbol);
-        MSVCRT_free(locinfo->lconv->currency_symbol);
-        MSVCRT_free(locinfo->lconv->mon_decimal_point);
-        MSVCRT_free(locinfo->lconv->mon_thousands_sep);
-        MSVCRT_free(locinfo->lconv->mon_grouping);
-        MSVCRT_free(locinfo->lconv->positive_sign);
-        MSVCRT_free(locinfo->lconv->negative_sign);
+        if(locinfo->lconv_num_refcount &&
+            !InterlockedDecrement(locinfo->lconv_num_refcount)) {
+            MSVCRT_free(locinfo->lconv->decimal_point);
+            MSVCRT_free(locinfo->lconv->thousands_sep);
+            MSVCRT_free(locinfo->lconv->grouping);
 #if _MSVCR_VER >= 100
-        MSVCRT_free(locinfo->lconv->_W_decimal_point);
-        MSVCRT_free(locinfo->lconv->_W_thousands_sep);
-        MSVCRT_free(locinfo->lconv->_W_int_curr_symbol);
-        MSVCRT_free(locinfo->lconv->_W_currency_symbol);
-        MSVCRT_free(locinfo->lconv->_W_mon_decimal_point);
-        MSVCRT_free(locinfo->lconv->_W_mon_thousands_sep);
-        MSVCRT_free(locinfo->lconv->_W_positive_sign);
-        MSVCRT_free(locinfo->lconv->_W_negative_sign);
+            MSVCRT_free(locinfo->lconv->_W_decimal_point);
+            MSVCRT_free(locinfo->lconv->_W_thousands_sep);
 #endif
+            MSVCRT_free(locinfo->lconv_num_refcount);
+        }
+        if(locinfo->lconv_mon_refcount &&
+            !InterlockedDecrement(locinfo->lconv_mon_refcount)) {
+            MSVCRT_free(locinfo->lconv->int_curr_symbol);
+            MSVCRT_free(locinfo->lconv->currency_symbol);
+            MSVCRT_free(locinfo->lconv->mon_decimal_point);
+            MSVCRT_free(locinfo->lconv->mon_thousands_sep);
+            MSVCRT_free(locinfo->lconv->mon_grouping);
+            MSVCRT_free(locinfo->lconv->positive_sign);
+            MSVCRT_free(locinfo->lconv->negative_sign);
+#if _MSVCR_VER >= 100
+            MSVCRT_free(locinfo->lconv->_W_int_curr_symbol);
+            MSVCRT_free(locinfo->lconv->_W_currency_symbol);
+            MSVCRT_free(locinfo->lconv->_W_mon_decimal_point);
+            MSVCRT_free(locinfo->lconv->_W_mon_thousands_sep);
+            MSVCRT_free(locinfo->lconv->_W_positive_sign);
+            MSVCRT_free(locinfo->lconv->_W_negative_sign);
+#endif
+            MSVCRT_free(locinfo->lconv_mon_refcount);
+        }
+        if(locinfo->lconv_intl_refcount &&
+            !InterlockedDecrement(locinfo->lconv_intl_refcount)) {
+            MSVCRT_free(locinfo->lconv_intl_refcount);
+            MSVCRT_free(locinfo->lconv);
+        }
     }
-    MSVCRT_free(locinfo->lconv_intl_refcount);
-    MSVCRT_free(locinfo->lconv_num_refcount);
-    MSVCRT_free(locinfo->lconv_mon_refcount);
-    MSVCRT_free(locinfo->lconv);
 
-    MSVCRT_free(locinfo->ctype1_refcount);
-    MSVCRT_free(locinfo->ctype1);
+    if(locinfo->ctype1_refcount &&
+        !InterlockedDecrement(locinfo->ctype1_refcount)) {
+        MSVCRT_free(locinfo->ctype1_refcount);
+        MSVCRT_free(locinfo->ctype1);
 
-    MSVCRT_free(locinfo->pclmap);
-    MSVCRT_free(locinfo->pcumap);
+        MSVCRT_free(locinfo->pclmap);
+        MSVCRT_free(locinfo->pcumap);
+    }
 
     if(locinfo->lc_time_curr != &cloc_time_data)
         MSVCRT_free(locinfo->lc_time_curr);
@@ -939,14 +956,34 @@ void free_mbcinfo(MSVCRT_pthreadmbcinfo mbcinfo)
  */
 MSVCRT__locale_t CDECL MSVCRT__get_current_locale(void)
 {
+    int i;
     MSVCRT__locale_t loc = MSVCRT_malloc(sizeof(MSVCRT__locale_tstruct));
     if(!loc)
         return NULL;
 
-    loc->locinfo = get_locinfo();
-    loc->mbcinfo = get_mbcinfo();
-    InterlockedIncrement(&loc->locinfo->refcount);
-    InterlockedIncrement(&loc->mbcinfo->refcount);
+    loc->locinfo = MSVCRT_malloc(sizeof(MSVCRT_threadlocinfo));
+    loc->mbcinfo = MSVCRT_malloc(sizeof(MSVCRT_threadmbcinfo));
+    if (!loc->locinfo || !loc->mbcinfo) {
+        MSVCRT_free(loc->locinfo);
+        MSVCRT_free(loc->mbcinfo);
+        MSVCRT_free(loc);
+        return NULL;
+    }
+    memcpy(loc->locinfo, get_locinfo(), sizeof(MSVCRT_threadlocinfo));
+    memcpy(loc->mbcinfo, get_mbcinfo(), sizeof(MSVCRT_threadmbcinfo));
+    loc->locinfo->refcount = 1;
+    loc->mbcinfo->refcount = 1;
+    for(i = 1; i <= (MSVCRT_LC_MAX - MSVCRT_LC_MIN); i++)
+        if(loc->locinfo->lc_category[i].refcount)
+            InterlockedIncrement(loc->locinfo->lc_category[i].refcount);
+    if(loc->locinfo->lconv_intl_refcount)
+        InterlockedIncrement(loc->locinfo->lconv_intl_refcount);
+    if(loc->locinfo->lconv_num_refcount)
+        InterlockedIncrement(loc->locinfo->lconv_num_refcount);
+    if(loc->locinfo->lconv_mon_refcount)
+        InterlockedIncrement(loc->locinfo->lconv_mon_refcount);
+    if(loc->locinfo->ctype1_refcount)
+        InterlockedIncrement(loc->locinfo->ctype1_refcount);
     return loc;
 }
 
@@ -1213,8 +1250,12 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
             free_locinfo(locinfo);
             return NULL;
         }
-    } else
-        locinfo->lc_category[MSVCRT_LC_COLLATE].locale = MSVCRT__strdup("C");
+    } else {
+        if(!init_category_name("C", 1, locinfo, MSVCRT_LC_COLLATE)) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
+    }
 
     if(locale_name[MSVCRT_LC_CTYPE] &&
             !init_category_name(locale_name[MSVCRT_LC_CTYPE],
@@ -1291,7 +1332,10 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
         locinfo->lc_clike = 1;
         locinfo->mb_cur_max = 1;
         locinfo->pctype = MSVCRT__ctype+1;
-        locinfo->lc_category[MSVCRT_LC_CTYPE].locale = MSVCRT__strdup("C");
+        if(!init_category_name("C", 1, locinfo, MSVCRT_LC_CTYPE)) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
 
         for(i=0; i<256; i++) {
             if(locinfo->pctype[i] & MSVCRT__LEADBYTE)
@@ -1528,6 +1572,16 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
             return NULL;
         }
     } else {
+        locinfo->lconv_intl_refcount = MSVCRT_malloc(sizeof(int));
+        locinfo->lconv_mon_refcount = MSVCRT_malloc(sizeof(int));
+        if(!locinfo->lconv_intl_refcount || !locinfo->lconv_mon_refcount) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
+
+        *locinfo->lconv_intl_refcount = 1;
+        *locinfo->lconv_mon_refcount = 1;
+
         locinfo->lconv->int_curr_symbol = MSVCRT_malloc(sizeof(char));
         locinfo->lconv->currency_symbol = MSVCRT_malloc(sizeof(char));
         locinfo->lconv->mon_decimal_point = MSVCRT_malloc(sizeof(char));
@@ -1583,7 +1637,10 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
         locinfo->lconv->_W_negative_sign[0] = '\0';
 #endif
 
-        locinfo->lc_category[MSVCRT_LC_MONETARY].locale = MSVCRT__strdup("C");
+        if(!init_category_name("C", 1, locinfo, MSVCRT_LC_MONETARY)) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
     }
 
     if(locale_name[MSVCRT_LC_NUMERIC] &&
@@ -1673,6 +1730,17 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
             return NULL;
         }
     } else {
+        if(!locinfo->lconv_intl_refcount)
+            locinfo->lconv_intl_refcount = MSVCRT_malloc(sizeof(int));
+        locinfo->lconv_num_refcount = MSVCRT_malloc(sizeof(int));
+        if(!locinfo->lconv_intl_refcount || !locinfo->lconv_num_refcount) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
+
+        *locinfo->lconv_intl_refcount = 1;
+        *locinfo->lconv_num_refcount = 1;
+
         locinfo->lconv->decimal_point = MSVCRT_malloc(sizeof(char[2]));
         locinfo->lconv->thousands_sep = MSVCRT_malloc(sizeof(char));
         locinfo->lconv->grouping = MSVCRT_malloc(sizeof(char));
@@ -1701,7 +1769,10 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
         locinfo->lconv->_W_thousands_sep[0] = '\0';
 #endif
 
-        locinfo->lc_category[MSVCRT_LC_NUMERIC].locale = MSVCRT__strdup("C");
+        if (!init_category_name("C", 1, locinfo, MSVCRT_LC_NUMERIC)) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
     }
 
     if(locale_name[MSVCRT_LC_TIME] &&
@@ -1733,7 +1804,10 @@ static MSVCRT_pthreadlocinfo create_locinfo(int category,
             return NULL;
         }
     } else {
-        locinfo->lc_category[MSVCRT_LC_TIME].locale = MSVCRT__strdup("C");
+        if(!init_category_name("C", 1, locinfo, MSVCRT_LC_TIME)) {
+            free_locinfo(locinfo);
+            return NULL;
+        }
         locinfo->lc_time_curr = &cloc_time_data;
     }
 
-- 
2.24.0

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=63854

Your paranoid android.


=== build (build log) ===

error: patch failed: dlls/msvcr90/tests/msvcr90.c:139
Task: Patch failed to apply

=== debian10 (build log) ===

error: patch failed: dlls/msvcr90/tests/msvcr90.c:139
Task: Patch failed to apply

=== debian10 (build log) ===

error: patch failed: dlls/msvcr90/tests/msvcr90.c:139
Task: Patch failed to apply

[Piotr Caban]

Hi Chip,

On 1/27/20 3:18 AM, Chip Davis wrote:
> MSVCRT__locale_t CDECL MSVCRT__get_current_locale(void)
>   {
> +    int i;
>       MSVCRT__locale_t loc = MSVCRT_malloc(sizeof(MSVCRT__locale_tstruct));
>       if(!loc)
>           return NULL;
>
> -    loc->locinfo = get_locinfo();
> -    loc->mbcinfo = get_mbcinfo();
> -    InterlockedIncrement(&loc->locinfo->refcount);
> -    InterlockedIncrement(&loc->mbcinfo->refcount);
> +    loc->locinfo = MSVCRT_malloc(sizeof(MSVCRT_threadlocinfo));
> +    loc->mbcinfo = MSVCRT_malloc(sizeof(MSVCRT_threadmbcinfo));
> A quick testing shows that consecutive  _get_current_locale calls return 
> identical locinfo/mbcinfo pointers. It's probably the setlocale function 
> that needs to not reuse some strctures when refcount was icreased.

Thanks,
Piotr