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

src/common/xlocale.cpp

@@ -279,70 +279,91 @@ int wxToupper_l(const wxUniChar& c, const wxXLocale& loc)
              case !wxHAS_XLOCALE_SUPPORT...
 */
 
-/*
+namespace
-    Note that this code is similar to (a portion of) wxLocale::IsAvailable code
+{
-*/
+
-#define IMPLEMENT_STRTOX_L_START                                \
+// Helper class that changes LC_NUMERIC facet of the global locale in its ctor
-    wxCHECK(loc.IsOk(), 0);                                     \
+// to "C" locale and restores it in its dtor later.
-                                                                \
+class CNumericLocaleSetter
-    /* (Try to) temporary set the 'C' locale */                 \
+{
-    const char *oldLocale = wxSetlocale(LC_NUMERIC, "C");       \
+public:
-    if ( !oldLocale )                                           \
+    CNumericLocaleSetter()
-    {                                                           \
+        : m_oldLocale(wxStrdupA(setlocale(LC_NUMERIC, NULL)))
-        /* the current locale was not changed; no need to */    \
+    {
-        /* restore the previous one... */                       \
+        if ( !wxSetlocale(LC_NUMERIC, "C") )
-        errno = EINVAL;                                         \
+        {
-            /* signal an error (better than nothing) */         \
+            // Setting locale to "C" should really always work.
-        return 0;                                               \
+            wxFAIL_MSG( wxS("Couldn't set LC_NUMERIC to \"C\"") );
+        }
     }
 
-#define IMPLEMENT_STRTOX_L_END                                  \
+    ~CNumericLocaleSetter()
-    /* restore the original locale */                           \
+    {
-    wxSetlocale(LC_NUMERIC, oldLocale);                         \
+        wxSetlocale(LC_NUMERIC, m_oldLocale);
-    return ret;
+        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
+    wxCHECK( loc.IsOk(), 0. );
-    double ret = wxStrtod(str, endptr);
+
-    IMPLEMENT_STRTOX_L_END
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtod(str, endptr);
 }
 
 double wxStrtod_l(const char* str, char **endptr, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
+    wxCHECK( loc.IsOk(), 0. );
-    double ret = wxStrtod(str, endptr);
+
-    IMPLEMENT_STRTOX_L_END
+    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
+    wxCHECK( loc.IsOk(), 0 );
-    long ret = wxStrtol(str, endptr, base);
+
-    IMPLEMENT_STRTOX_L_END
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtol(str, endptr, base);
 }
 
 long wxStrtol_l(const char* str, char **endptr, int base, const wxXLocale& loc)
 {
-    IMPLEMENT_STRTOX_L_START
+    wxCHECK( loc.IsOk(), 0 );
-    long ret = wxStrtol(str, endptr, base);
+
-    IMPLEMENT_STRTOX_L_END
+    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
+    wxCHECK( loc.IsOk(), 0 );
-    unsigned long ret = wxStrtoul(str, endptr, base);
+
-    IMPLEMENT_STRTOX_L_END
+    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
+    wxCHECK( loc.IsOk(), 0 );
-    unsigned long ret = wxStrtoul(str, endptr, base);
-    IMPLEMENT_STRTOX_L_END
-}
 
+    CNumericLocaleSetter locSetter;
+
+    return wxStrtoul(str, endptr, base);
+}
 
 #endif // !defined(wxHAS_XLOCALE_SUPPORT)
 

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)
 {