diff mbox

PR 13631 Problems in messages

Message ID 20150318161744.GT9755@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely March 18, 2015, 4:17 p.m. UTC 
On 03/12/14 14:54 +0000, Jonathan Wakely wrote:
>On 02/12/14 23:58 +0100, François Dumont wrote:
>>
>>   DR libstdc++/13631
>
>s/DR/PR/
>
>>   * include/bits/codecvt.h (codecvt<char, char, mbstate_t>): friend class
>>   std::messages<char>.
>>   (codecvt<wchar_t, char, mbstate_t>): friend class
>>   std::messages<wchar_t>.
>>   * config/locale/gnu/messages_member.h
>>   (messages<char>::do_open): Specialized.
>>   (messages<char>::do_close): Likewise.
>>   (messages<wchar_t>::do_open): Likewise.
>>   (messages<wchar_t>::do_close): Likewise.
>>   * config/locale/gnu/messages_member.cc:
>>   (messages<char>::do_open): Implement. Use bind_textdomain_codeset based
>>   on codecvt<char, char, mbstate_t>._M_c_locale_codecvt code set. Use
>>   internal cache to keep opened domain name with locale information.
>>   (messages<wchar_t>::do_open): Likewise with
>>   codecvt<wchar_t, char, mbstate_t>.
>>   (messages<char>::do_close): Implement. Clean cache information.
>>   (messages<wchar_t>::do_close): Likewise.
>>   (get_glibc_msg): New. Use dgettext rather than gettext using cached
>>   domain name associated to catalog id.
>>   (messages<char>::do_get): Use latter.
>>   (messages<wchar_t>::do_get): Likewise and use also cached locale
>>   codecvt<wchar_t, char, mbstate_t> facet to convert wchar_t default
>>   value to char and the result back to wchar_t.
>>   * testsuite/22_locale/messages/13631.cc: New.
>>   * testsuite/22_locale/messages/members/char/2.cc: Use also fr_FR locale
>>   for charset conversion to get the expected accented character.
>>
>>Tested under Linux x86_64.
>>
>>Ok to commit ?
>
>Yes, thanks for fixing this longstanding bug!
>
>(The use of std::string might have to change when we have two
>different versions of std::string, as we probably only want one
>Catalogs object to be shared by both ABIs, but I will deal with that
>as part of the std::string patch)

The new get_glibc_msg function doesn't compile with old versions of
glibc, _M_name_messages is not defined (the code is also not exception
safe, but that was not introduced by the recent changes).

Tested x86_64-linux, committed to trunk.

Comments

Jonathan Wakely March 18, 2015, 4:31 p.m. UTC | #1 
On 18/03/15 16:17 +0000, Jonathan Wakely wrote:
>The new get_glibc_msg function doesn't compile with old versions of
>glibc, _M_name_messages is not defined (the code is also not exception
>safe, but that was not introduced by the recent changes).

We can probably drop that old code completely, given that we document
that glibc 2.3 is required:
https://gcc.gnu.org/onlinedocs/libstdc++/manual/setup.html#manual.intro.setup.prereq

Let's look into that for GCC 6.
diff mbox

Patch

commit eb85799122e4aaecfc5a231a3f012b5dbbf88116
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Mar 18 15:56:12 2015 +0000

    	PR libstdc++/13631
    	* config/locale/gnu/messages_members.cc (get_glibc_msg): Fix fallback
    	implementation for old glibc. Fix whitespace.

diff --git a/libstdc++-v3/config/locale/gnu/messages_members.cc b/libstdc++-v3/config/locale/gnu/messages_members.cc
index f115d5f..2e6122d 100644
--- a/libstdc++-v3/config/locale/gnu/messages_members.cc
+++ b/libstdc++-v3/config/locale/gnu/messages_members.cc
@@ -34,6 +34,8 @@ 
 #include <limits>
 #include <algorithm>
 #include <vector>
+#include <cstdlib>	// std::free
+#include <string.h>	// ::strdup
 
 #include <backward/auto_ptr.h>
 #include <ext/concurrence.h>
@@ -139,28 +141,28 @@  namespace
   }
 
   const char*
-  get_glibc_msg(__c_locale __attribute__((unused)) __locale_messages,
+  get_glibc_msg(__c_locale __locale_messages __attribute__((unused)),
+		const char* __name_messages __attribute__((unused)),
 		const char* __domainname,
 		const char* __dfault)
   {
 #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ > 2)
     std::__c_locale __old = __uselocale(__locale_messages);
-    const char* __msg =
-      const_cast<const char*>(dgettext(__domainname, __dfault));
-      __uselocale(__old);
-#else
-      char* __old = setlocale(LC_ALL, 0);
-      const size_t __len = strlen(__old) + 1;
-      char* __sav = new char[__len];
-      memcpy(__sav, __old, __len);
-      setlocale(LC_ALL, _M_name_messages);
     const char* __msg = dgettext(__domainname, __dfault);
-      setlocale(LC_ALL, __sav);
-      delete [] __sav;
-#endif
-
+    __uselocale(__old);
     return __msg;
-    }
+#else
+    if (char* __sav = strdup(setlocale(LC_ALL, 0)))
+      {
+	setlocale(LC_ALL, __name_messages);
+	const char* __msg = dgettext(__domainname, __dfault);
+	setlocale(LC_ALL, __sav);
+	free(__sav);
+	return __msg;
+      }
+    return __dfault;
+#endif
+  }
 }
 
 namespace std _GLIBCXX_VISIBILITY(default)
@@ -172,14 +174,14 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     typename messages<char>::catalog
     messages<char>::do_open(const basic_string<char>& __s,
 			    const locale& __l) const
-  {
-    typedef codecvt<char, char, mbstate_t> __codecvt_t;
-    const __codecvt_t& __codecvt = use_facet<__codecvt_t>(__l);
+    {
+      typedef codecvt<char, char, mbstate_t> __codecvt_t;
+      const __codecvt_t& __codecvt = use_facet<__codecvt_t>(__l);
 
-    bind_textdomain_codeset(__s.c_str(),
-	__nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
-    return get_catalogs()._M_add(__s, __l);
-  }
+      bind_textdomain_codeset(__s.c_str(),
+	  __nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
+      return get_catalogs()._M_add(__s, __l);
+    }
 
   template<>
     void
@@ -199,7 +201,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       if (!__cat_info)
 	return __dfault;
 
-      return get_glibc_msg(_M_c_locale_messages,
+      return get_glibc_msg(_M_c_locale_messages, _M_name_messages,
 			   __cat_info->_M_domain.c_str(),
 			   __dfault.c_str());
     }
@@ -209,15 +211,15 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     typename messages<wchar_t>::catalog
     messages<wchar_t>::do_open(const basic_string<char>& __s,
 			       const locale& __l) const
-  {
-    typedef codecvt<wchar_t, char, mbstate_t> __codecvt_t;
-    const __codecvt_t& __codecvt = use_facet<__codecvt_t>(__l);
+    {
+      typedef codecvt<wchar_t, char, mbstate_t> __codecvt_t;
+      const __codecvt_t& __codecvt = use_facet<__codecvt_t>(__l);
 
-    bind_textdomain_codeset(__s.c_str(),
-	__nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
+      bind_textdomain_codeset(__s.c_str(),
+	  __nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
 
-    return get_catalogs()._M_add(__s, __l);
-  }
+      return get_catalogs()._M_add(__s, __l);
+    }
 
   template<>
     void
@@ -257,15 +259,14 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 	// Make sure string passed to dgettext is \0 terminated.
 	*__dfault_next = '\0';
-	__translation
-	  = get_glibc_msg(_M_c_locale_messages,
-			  __cat_info->_M_domain.c_str(), __dfault);
+	__translation = get_glibc_msg(_M_c_locale_messages, _M_name_messages,
+				      __cat_info->_M_domain.c_str(), __dfault);
 
 	// If we end up getting default value back we can simply return original
 	// default value.
 	if (__translation == __dfault)
 	  return __wdfault;
-    }
+      }
 
       __builtin_memset(&__state, 0, sizeof(mbstate_t));
       size_t __size = __builtin_strlen(__translation);