From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Wed, 6 Apr 2011 14:37:32 +0000 (+0000)
Subject: Correctly restore the old locale in wxXLocale functions.
X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/cee25be77b0a5678011b45c22c60b65c0726fe8b

Correctly restore the old locale in wxXLocale functions.

In non-wxHAS_XLOCALE_SUPPORT case we didn't restore the original locale
correctly in wxStrtoxxx_l() functions as the return value of wxSetlocale() was
incorrectly assumed to be the old locale instead of the new one.

Fix this and also replace the macros used by the old code with a small helper
class, this simplifies the code and is less ugly.

Finally add a unit test which failed before these changes when the program ran
in any non-C locale but passes now.

Closes #13117.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@67406 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---

diff --git a/src/common/xlocale.cpp b/src/common/xlocale.cpp
index b1c08bade9..d0197463f8 100644
--- a/src/common/xlocale.cpp
+++ b/src/common/xlocale.cpp
@@ -279,70 +279,91 @@ int wxToupper_l(const wxUniChar& c, const wxXLocale& loc)
              case !wxHAS_XLOCALE_SUPPORT...
 */
 
-/*
-    Note that this code is similar to (a portion of) wxLocale::IsAvailable code
-*/
-#define IMPLEMENT_STRTOX_L_START                                \
-    wxCHECK(loc.IsOk(), 0);                                     \
-                                                                \
-    /* (Try to) temporary set the 'C' locale */                 \
-    const char *oldLocale = wxSetlocale(LC_NUMERIC, "C");       \
-    if ( !oldLocale )                                           \
-    {                                                           \
-        /* the current locale was not changed; no need to */    \
-        /* restore the previous one... */                       \
-        errno = EINVAL;                                         \
-            /* signal an error (better than nothing) */         \
-        return 0;                                               \
+namespace
+{
+
+// Helper class that changes LC_NUMERIC facet of the global locale in its ctor
+// to "C" locale and restores it in its dtor later.
+class CNumericLocaleSetter
+{
+public:
+    CNumericLocaleSetter()
+        : m_oldLocale(wxStrdupA(setlocale(LC_NUMERIC, NULL)))
+    {
+        if ( !wxSetlocale(LC_NUMERIC, "C") )
+        {
+            // Setting locale to "C" should really always work.
+            wxFAIL_MSG( wxS("Couldn't set LC_NUMERIC to \"C\"") );
+        }
     }
 
-#define IMPLEMENT_STRTOX_L_END                                  \
-    /* restore the original locale */                           \
-    wxSetlocale(LC_NUMERIC, oldLocale);                         \
-    return ret;
+    ~CNumericLocaleSetter()
+    {
+        wxSetlocale(LC_NUMERIC, m_oldLocale);
+        free(m_oldLocale);
+    }
+
+private:
+    char * const m_oldLocale;
+
+    wxDECLARE_NO_COPY_CLASS(CNumericLocaleSetter);
+};
+
+} // anonymous namespace
 
 double wxStrtod_l(const wchar_t* str, wchar_t **endptr, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    double ret = wxStrtod(str, endptr);
-    IMPLEMENT_STRTOX_L_END
+    wxCHECK( loc.IsOk(), 0. );
+
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtod(str, endptr);
 }
 
 double wxStrtod_l(const char* str, char **endptr, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    double ret = wxStrtod(str, endptr);
-    IMPLEMENT_STRTOX_L_END
+    wxCHECK( loc.IsOk(), 0. );
+
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtod(str, endptr);
 }
 
 long wxStrtol_l(const wchar_t* str, wchar_t **endptr, int base, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    long ret = wxStrtol(str, endptr, base);
-    IMPLEMENT_STRTOX_L_END
+    wxCHECK( loc.IsOk(), 0 );
+
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtol(str, endptr, base);
 }
 
 long wxStrtol_l(const char* str, char **endptr, int base, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    long ret = wxStrtol(str, endptr, base);
-    IMPLEMENT_STRTOX_L_END
+    wxCHECK( loc.IsOk(), 0 );
+
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtol(str, endptr, base);
 }
 
 unsigned long wxStrtoul_l(const wchar_t* str, wchar_t **endptr, int base, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    unsigned long ret = wxStrtoul(str, endptr, base);
-    IMPLEMENT_STRTOX_L_END
+    wxCHECK( loc.IsOk(), 0 );
+
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtoul(str, endptr, base);
 }
 
 unsigned long wxStrtoul_l(const char* str, char **endptr, int base, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
-    unsigned long ret = wxStrtoul(str, endptr, base);
-    IMPLEMENT_STRTOX_L_END
-}
+    wxCHECK( loc.IsOk(), 0 );
+
+    CNumericLocaleSetter locSetter;
 
+    return wxStrtoul(str, endptr, base);
+}
 
 #endif // !defined(wxHAS_XLOCALE_SUPPORT)
 
diff --git a/tests/xlocale/xlocale.cpp b/tests/xlocale/xlocale.cpp
index 9feddf9a2c..b9b4ef7b71 100644
--- a/tests/xlocale/xlocale.cpp
+++ b/tests/xlocale/xlocale.cpp
@@ -39,11 +39,13 @@ public:
 private:
     CPPUNIT_TEST_SUITE( XLocaleTestCase );
         CPPUNIT_TEST( TestCtor );
+        CPPUNIT_TEST( PreserveLocale );
         CPPUNIT_TEST( TestCtypeFunctions );
         CPPUNIT_TEST( TestStdlibFunctions );
     CPPUNIT_TEST_SUITE_END();
 
     void TestCtor();
+    void PreserveLocale();
     void TestCtypeFunctions();
     void TestStdlibFunctions();
 
@@ -81,6 +83,16 @@ void XLocaleTestCase::TestCtor()
 #endif
 }
 
+void XLocaleTestCase::PreserveLocale()
+{
+    // Test that using locale functions doesn't change the global C locale.
+    const wxString origLocale(setlocale(LC_ALL, NULL));
+
+    wxStrtod_l(wxT("1.234"), NULL, wxCLocale);
+
+    CPPUNIT_ASSERT_EQUAL( origLocale, setlocale(LC_ALL, NULL) );
+}
+
 // test the ctype functions with the given locale
 void XLocaleTestCase::TestCtypeFunctionsWith(const wxXLocale& loc)
 {