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 b1c08bade9cb215dfa0c2a249f563af9e3203350..d0197463f802cd6ad11a7296ccf3cc462f409384 100644 (file)
--- 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 9feddf9a2ccae83e6b21aea955d312467b488690..b9b4ef7b7121b95f908415f91ac79744dc1b5ed9 100644 (file)
--- 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)
 {