From 5e429702bfa1dd62ffcd26031f5d155e3c3870a8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 Dec 2017 00:03:39 +0100 Subject: [PATCH 1/6] Fix fatal bug in wxXLocale initialization Ensure that m_locale is always initialized to null pointer, as it could remain not initialized at all if information for wxLanguage could be found, but its locale name was empty, which resulted in a crash in dtor when the wxXLocale object was destroyed as freelocale() was called with an invalid pointer. --- src/common/xlocale.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/common/xlocale.cpp b/src/common/xlocale.cpp index 02c9d2ca2d..e8095a1132 100644 --- a/src/common/xlocale.cpp +++ b/src/common/xlocale.cpp @@ -90,12 +90,10 @@ wxXLocale& wxXLocale::GetCLocale() #if wxUSE_INTL wxXLocale::wxXLocale(wxLanguage lang) { + m_locale = NULL; + const wxLanguageInfo * const info = wxLocale::GetLanguageInfo(lang); - if ( !info ) - { - m_locale = NULL; - } - else + if ( info ) { Init(info->GetLocaleName().c_str()); } From 17041075b40f39c4e1cabc7b5155e2506fa65ed5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 Dec 2017 00:10:38 +0100 Subject: [PATCH 2/6] Fix wxLanguageInfo::GetLocaleName() This function could (and did) return completely wrong results because the value of the pointer returned by setlocale() could be (and was) changed by the subsequent call to setlocale() with different arguments. Fix the problem by copying the returned value into wxString immediately, without any intervening setlocale() calls. --- src/common/intl.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/common/intl.cpp b/src/common/intl.cpp index 1fc2b8d989..96dd2389d6 100644 --- a/src/common/intl.cpp +++ b/src/common/intl.cpp @@ -208,12 +208,17 @@ wxString wxLanguageInfo::GetLocaleName() const const char* const orig = wxSetlocale(LC_ALL, NULL); const char* const ret = TrySetLocale(); - if ( !ret ) - return wxString(); + wxString retval; + if ( ret ) + { + // Note that we must copy the returned value before calling setlocale() + // again as the string "ret" points to can (and, at least under Linux + // with glibc, actually always will) be changed by this call. + retval = ret; + wxSetlocale(LC_ALL, orig); + } - wxSetlocale(LC_ALL, orig); - - return ret; + return retval; } // ---------------------------------------------------------------------------- From 38cc8498d1814380127a3c3669ebbdfe1f619268 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 Dec 2017 00:18:38 +0100 Subject: [PATCH 3/6] Fix wxXLocale availability detection in configure This was broken by 9507bc430e2faf16b589958be91a462ca8e69813 which stopped defining HAVE_LOCALE_T in configure but didn't update the code using it. Restore the old behaviour by continuing to define HAVE_LOCALE_T even if we don't test (just) for it any longer. See https://github.com/wxWidgets/wxWidgets/pull/461 --- configure | 5 ++++- configure.in | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/configure b/configure index fa8141305a..ca5b78ace9 100755 --- a/configure +++ b/configure @@ -32627,6 +32627,9 @@ $as_echo "$as_me: WARNING: I18n code requires wxFile... disabled" >&2;} fi if test "$wxUSE_XLOCALE" = "yes" ; then + $as_echo "#define wxUSE_XLOCALE 1" >>confdefs.h + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for complete xlocale" >&5 $as_echo_n "checking for complete xlocale... " >&6; } if ${wx_cv_func_strtod_l+:} false; then : @@ -32676,7 +32679,7 @@ fi $as_echo "$wx_cv_func_strtod_l" >&6; } if test "$wx_cv_func_strtod_l" = "yes" ; then - $as_echo "#define wxUSE_XLOCALE 1" >>confdefs.h + $as_echo "#define HAVE_LOCALE_T 1" >>confdefs.h fi fi diff --git a/configure.in b/configure.in index 9b0bbd969e..8eb3571261 100644 --- a/configure.in +++ b/configure.in @@ -5747,6 +5747,8 @@ if test "$wxUSE_INTL" = "yes" ; then fi if test "$wxUSE_XLOCALE" = "yes" ; then + AC_DEFINE(wxUSE_XLOCALE) + dnl even if xlocale.h exists, it may not contain all that dnl wx needs. check if strtod_l() really is available. AC_CACHE_CHECK([for complete xlocale], @@ -5770,7 +5772,9 @@ if test "$wxUSE_XLOCALE" = "yes" ; then ]) if test "$wx_cv_func_strtod_l" = "yes" ; then - AC_DEFINE(wxUSE_XLOCALE) + dnl We don't test (just) for locale_t existence, but we still define + dnl this symbol to avoid changing the existing code using it. + AC_DEFINE(HAVE_LOCALE_T) fi fi From e97c020285c785464373a9854d9f9d824777228f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 9 Dec 2017 19:02:16 -0700 Subject: [PATCH 4/6] Fix wxLocale::GetInfo() test for French locale under macOS 10.12 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The date and time format has changed since 10.10 and now contains an extra " à " in its middle, so adjust the test to deal with this. --- tests/intl/intltest.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/intl/intltest.cpp b/tests/intl/intltest.cpp index ecf8770640..6825f8a20a 100644 --- a/tests/intl/intltest.cpp +++ b/tests/intl/intltest.cpp @@ -189,19 +189,25 @@ void IntlTestCase::DateTimeFmtFrench() #else static const char *FRENCH_DATE_FMT = "%d/%m/%Y"; static const char *FRENCH_LONG_DATE_FMT = "%A %d %B %Y"; -#ifdef __WXOSX__ - static const char *FRENCH_DATE_TIME_FMT = "%A %d %B %Y %H:%M:%S"; -#else static const char *FRENCH_DATE_TIME_FMT = "%d/%m/%Y %H:%M:%S"; -#endif #endif WX_ASSERT_EQUAL_FORMAT( "French short date", FRENCH_DATE_FMT, m_locale->GetInfo(wxLOCALE_SHORT_DATE_FMT) ); WX_ASSERT_EQUAL_FORMAT( "French long date", FRENCH_LONG_DATE_FMT, m_locale->GetInfo(wxLOCALE_LONG_DATE_FMT) ); - WX_ASSERT_EQUAL_FORMAT( "French date and time", FRENCH_DATE_TIME_FMT, - m_locale->GetInfo(wxLOCALE_DATE_TIME_FMT) ); + + const wxString fmtDT = m_locale->GetInfo(wxLOCALE_DATE_TIME_FMT); +#ifdef __WXOSX__ + // Things are difficult to test under macOS as the format keeps changing, + // e.g. at some time between 10.10 and 10.12 a new " à " string appeared in + // its middle, so test it piece-wise and hope it doesn't change too much. + INFO("French date and time format is \"" << fmtDT << "\""); + CHECK( fmtDT.StartsWith("%A %d %B %Y") ); + CHECK( fmtDT.EndsWith("%H:%M:%S") ); +#else + WX_ASSERT_EQUAL_FORMAT( "French date and time", FRENCH_DATE_TIME_FMT, fmtDT ); +#endif WX_ASSERT_EQUAL_FORMAT( "French time", "%H:%M:%S", m_locale->GetInfo(wxLOCALE_TIME_FMT) ); } From 9036b3dba876131085251a18c2c0563fc86b7da0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 9 Dec 2017 19:16:47 -0700 Subject: [PATCH 5/6] Remove "C" locale date and time formats test This test didn't make sense at all as it didn't actually test "C" locale formats as calling setlocale(LC_ALL, "C") didn't actually change the values returned by wxLocale::GetInfo(), so it still returned the values corresponding to the French locale set in this test setUp() method and the test only passed because it used wrong values (i.e. the same ones as in French locale test). We also don't have any simple way to test "C" locale formats, we can only test them for wxLANGUAGE_DEFAULT, but this corresponds to the OS defaults which can be customized by user (e.g. in the control panel under MSW) and so we can't expect them to be equal to any fixed values. The simplest solution is to just drop this test, as it's not very useful anyhow (French locale test above already covers the same code). --- tests/intl/intltest.cpp | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/tests/intl/intltest.cpp b/tests/intl/intltest.cpp index 6825f8a20a..38a7df1a1c 100644 --- a/tests/intl/intltest.cpp +++ b/tests/intl/intltest.cpp @@ -42,7 +42,6 @@ private: CPPUNIT_TEST( Domain ); CPPUNIT_TEST( Headers ); CPPUNIT_TEST( DateTimeFmtFrench ); - CPPUNIT_TEST( DateTimeFmtC ); CPPUNIT_TEST( IsAvailable ); CPPUNIT_TEST_SUITE_END(); @@ -50,7 +49,6 @@ private: void Domain(); void Headers(); void DateTimeFmtFrench(); - void DateTimeFmtC(); void IsAvailable(); static wxString GetDecimalPoint() @@ -212,35 +210,6 @@ void IntlTestCase::DateTimeFmtFrench() m_locale->GetInfo(wxLOCALE_TIME_FMT) ); } -void IntlTestCase::DateTimeFmtC() -{ - // again, glibc uses different defaults -#ifdef __GLIBC__ - static const char *C_DATE_FMT = "%m/%d/%y"; - static const char *C_LONG_DATE_FMT = "%a %b %d %Y"; - static const char *C_DATE_TIME_FMT = "%a %b %d %H:%M:%S %Y"; -#else - static const char *C_DATE_FMT = "%d/%m/%Y"; - static const char *C_LONG_DATE_FMT = "%A %d %B %Y"; -#ifdef __WXOSX__ - static const char *C_DATE_TIME_FMT = "%A %d %B %Y %H:%M:%S"; -#else - static const char *C_DATE_TIME_FMT = "%d/%m/%Y %H:%M:%S"; -#endif -#endif - - setlocale(LC_ALL, "C"); - - WX_ASSERT_EQUAL_FORMAT( "C short date", C_DATE_FMT, - m_locale->GetInfo(wxLOCALE_SHORT_DATE_FMT) ); - WX_ASSERT_EQUAL_FORMAT( "C long date", C_LONG_DATE_FMT, - m_locale->GetInfo(wxLOCALE_LONG_DATE_FMT) ); - WX_ASSERT_EQUAL_FORMAT( "C date and time", C_DATE_TIME_FMT, - m_locale->GetInfo(wxLOCALE_DATE_TIME_FMT) ); - WX_ASSERT_EQUAL_FORMAT( "C time", "%H:%M:%S", - m_locale->GetInfo(wxLOCALE_TIME_FMT) ); -} - void IntlTestCase::IsAvailable() { const wxString origLocale(setlocale(LC_ALL, NULL)); From 2ad2bccf33c1d05136ee41cda903335c1c3c92e2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 Dec 2017 03:28:18 +0100 Subject: [PATCH 6/6] Don't call static wxLocale::GetInfo() via an object This is just confusing and unnecessary. No real changes. --- tests/intl/intltest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/intl/intltest.cpp b/tests/intl/intltest.cpp index 38a7df1a1c..4e39762a0f 100644 --- a/tests/intl/intltest.cpp +++ b/tests/intl/intltest.cpp @@ -191,11 +191,11 @@ void IntlTestCase::DateTimeFmtFrench() #endif WX_ASSERT_EQUAL_FORMAT( "French short date", FRENCH_DATE_FMT, - m_locale->GetInfo(wxLOCALE_SHORT_DATE_FMT) ); + wxLocale::GetInfo(wxLOCALE_SHORT_DATE_FMT) ); WX_ASSERT_EQUAL_FORMAT( "French long date", FRENCH_LONG_DATE_FMT, - m_locale->GetInfo(wxLOCALE_LONG_DATE_FMT) ); + wxLocale::GetInfo(wxLOCALE_LONG_DATE_FMT) ); - const wxString fmtDT = m_locale->GetInfo(wxLOCALE_DATE_TIME_FMT); + const wxString fmtDT = wxLocale::GetInfo(wxLOCALE_DATE_TIME_FMT); #ifdef __WXOSX__ // Things are difficult to test under macOS as the format keeps changing, // e.g. at some time between 10.10 and 10.12 a new " à " string appeared in @@ -207,7 +207,7 @@ void IntlTestCase::DateTimeFmtFrench() WX_ASSERT_EQUAL_FORMAT( "French date and time", FRENCH_DATE_TIME_FMT, fmtDT ); #endif WX_ASSERT_EQUAL_FORMAT( "French time", "%H:%M:%S", - m_locale->GetInfo(wxLOCALE_TIME_FMT) ); + wxLocale::GetInfo(wxLOCALE_TIME_FMT) ); } void IntlTestCase::IsAvailable()