[PATCH] oleaut32: Fix locale settings caching in VARIANT_GetLocalisedNumberChars().

Francois Gouget fgouget at codeweavers.com
Tue Jul 27 18:11:46 CDT 2021


Don't cache locale data coming from the registry because it could change
at any time.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
Previously the settings of the first VarParseNumFromStr(LOCALE_USER_DEFAULT)
call ended up being cached and used for all the remaining tests, thus
causing many of them to fail.

There's a comment that says the caching is because all these lookups are 
slow. And I guess the common case it likely to be the one that's no 
longer cached: i.e. getting the current user's preferences from the 
registry.

But all in all that case does not seem that slow (roughly 1.1 us per 
call) so unless it's called millions of times it should be ok. Also 
checking the registry key's last modification time would not make it 
much faster (I expect at most x2).
---
 dlls/oleaut32/tests/vartest.c | 12 +++---------
 dlls/oleaut32/variant.c       | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c
index bed4de469df..89859988f1f 100644
--- a/dlls/oleaut32/tests/vartest.c
+++ b/dlls/oleaut32/tests/vartest.c
@@ -2198,7 +2198,7 @@ static void test_VarParseNumFromStrMisc(void)
       /* But SMONDECIMALSEP has no default! */
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONDECIMALSEP, L"");
       hres = wconvert_str(L"3.9", ARRAY_SIZE(rgb), NUMPRS_DECIMAL|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECTFAIL;
+      EXPECTFAIL;
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, L".");
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONDECIMALSEP, L".");
 
@@ -2206,9 +2206,7 @@ static void test_VarParseNumFromStrMisc(void)
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, L" ");
 
       hres = wconvert_str(L"1 000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2225,9 +2223,7 @@ static void test_VarParseNumFromStrMisc(void)
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONTHOUSANDSEP, L" ");
 
       hres = wconvert_str(L"1|000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2235,9 +2231,7 @@ static void test_VarParseNumFromStrMisc(void)
       EXPECTFAIL;
 
       hres = wconvert_str(L"1|000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2298,14 +2292,14 @@ static void test_VarParseNumFromStrMisc(void)
 
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, L"\xa0");
       hres = wconvert_str(L"1 000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
+      EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
 
       /* Regular thousands separators also have precedence over the currency ones */
       hres = wconvert_str(L"1\xa0\x30\x30\x30", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
+      EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c
index e4993de75a5..15fc7f9042e 100644
--- a/dlls/oleaut32/variant.c
+++ b/dlls/oleaut32/variant.c
@@ -1529,8 +1529,11 @@ static void VARIANT_GetLocalisedNumberChars(VARIANT_NUMBER_CHARS *lpChars, LCID
   EnterCriticalSection(&cache_cs);
 
   /* Asking for default locale entries is very expensive: It is a registry
-     server call. So cache one locally, as Microsoft does it too */
-  if(lcid == lastLcid && dwFlags == lastFlags)
+   * server call. So cache one locally, as Microsoft does it too.
+   * Note: Data in the registry could change at any time so only cache when
+   *       LOCALE_NOUSEROVERRIDE is used.
+   */
+  if (lctype && lcid == lastLcid && dwFlags == lastFlags)
   {
     memcpy(lpChars, &lastChars, sizeof(defaultChars));
     LeaveCriticalSection(&cache_cs);
@@ -1557,9 +1560,12 @@ static void VARIANT_GetLocalisedNumberChars(VARIANT_NUMBER_CHARS *lpChars, LCID
   TRACE("lcid 0x%x, cCurrencyLocal=%d,%d %s\n", lcid, lpChars->cCurrencyLocal,
         lpChars->cCurrencyLocal2, wine_dbgstr_w(buff));
 
-  memcpy(&lastChars, lpChars, sizeof(defaultChars));
-  lastLcid = lcid;
-  lastFlags = dwFlags;
+  if (lctype)
+  {
+    memcpy(&lastChars, lpChars, sizeof(defaultChars));
+    lastLcid = lcid;
+    lastFlags = dwFlags;
+  }
   LeaveCriticalSection(&cache_cs);
 }
 
-- 
2.20.1



More information about the wine-devel mailing list