re PR libstdc++/13631 (Problems in messages)

2014-12-03  François Dumont  <fdumont@gcc.gnu.org>

	PR libstdc++/13631
	* 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.

From-SVN: r218329

libstdc++-v3/ChangeLog

@@ -1,3 +1,33 @@
+2014-12-03  François Dumont  <fdumont@gcc.gnu.org>
+
+	PR libstdc++/13631
+	* 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.
+
 2014-12-03  Marek Polacek  <polacek@redhat.com>
 
 	* include/bits/regex_compiler.h (_S_cache_size): Multiply the

libstdc++-v3/config/locale/gnu/messages_members.cc

@@ -31,54 +31,253 @@
 #include <locale>
 #include <bits/c++locale_internal.h>
 
-namespace std _GLIBCXX_VISIBILITY(default)
+#include <limits>
-{
+#include <algorithm>
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
+#include <vector>
 
-  // Specializations.
+#include <backward/auto_ptr.h>
-  template<>
+#include <ext/concurrence.h>
-    string
+
-    messages<char>::do_get(catalog, int, int, const string& __dfault) const
+namespace
+{
+  using namespace std;
+
+  typedef messages_base::catalog catalog;
+
+  struct Catalog_info
     {
+    Catalog_info(catalog __id, const string& __domain, locale __loc)
+      : _M_id(__id), _M_domain(__domain), _M_locale(__loc)
+    { }
+
+    catalog _M_id;
+    string _M_domain;
+    locale _M_locale;
+  };
+
+  class Catalogs
+  {
+  public:
+    Catalogs() : _M_catalog_counter(0) { }
+
+    ~Catalogs()
+    {
+      for (vector<Catalog_info*>::iterator __it = _M_infos.begin();
+	   __it != _M_infos.end(); ++__it)
+	delete *__it;
+    }
+
+    catalog
+    _M_add(const string& __domain, locale __l)
+    {
+      __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+      // The counter is not likely to roll unless catalogs keep on being
+      // opened/closed which is consider as an application mistake for the
+      // moment.
+      if (_M_catalog_counter == numeric_limits<catalog>::max())
+	return -1;
+
+      std::auto_ptr<Catalog_info> info(new Catalog_info(_M_catalog_counter++,
+							__domain, __l));
+      _M_infos.push_back(info.get());
+      return info.release()->_M_id;
+    }
+
+    void
+    _M_erase(catalog __c)
+    {
+      __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+      vector<Catalog_info*>::iterator __res =
+	lower_bound(_M_infos.begin(), _M_infos.end(), __c, _Comp());
+      if (__res == _M_infos.end() || (*__res)->_M_id != __c)
+	return;
+
+      delete *__res;
+      _M_infos.erase(__res);
+
+      // Just in case closed catalog was the last open.
+      if (__c == _M_catalog_counter - 1)
+	--_M_catalog_counter;
+    }
+
+    const Catalog_info*
+    _M_get(catalog __c) const
+    {
+      __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+      vector<Catalog_info*>::const_iterator __res =
+	lower_bound(_M_infos.begin(), _M_infos.end(), __c, _Comp());
+
+      if (__res != _M_infos.end() && (*__res)->_M_id == __c)
+	return *__res;
+
+      return 0;
+    }
+
+  private:
+    struct _Comp
+    {
+      bool operator()(catalog __cat, const Catalog_info* __info) const
+      { return __cat < __info->_M_id; }
+
+      bool operator()(const Catalog_info* __info, catalog __cat) const
+      { return __info->_M_id < __cat; }
+    };
+
+    mutable __gnu_cxx::__mutex _M_mutex;
+    catalog _M_catalog_counter;
+    std::vector<Catalog_info*> _M_infos;
+  };
+
+  Catalogs&
+  get_catalogs()
+  {
+    static Catalogs __catalogs;
+    return __catalogs;
+  }
+
+  const char*
+  get_glibc_msg(__c_locale __attribute__((unused)) __locale_messages,
+		const char* __domainname,
+		const char* __dfault)
+  {
 #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ > 2)
-      __c_locale __old = __uselocale(_M_c_locale_messages);
+    std::__c_locale __old = __uselocale(__locale_messages);
-      const char* __msg = const_cast<const char*>(gettext(__dfault.c_str()));
+    const char* __msg =
+      const_cast<const char*>(dgettext(__domainname, __dfault));
       __uselocale(__old);
-      return string(__msg);
 #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 = gettext(__dfault.c_str());
+    const char* __msg = dgettext(__domainname, __dfault);
       setlocale(LC_ALL, __sav);
       delete [] __sav;
-      return string(__msg);
 #endif
+
+    return __msg;
+    }
+}
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  // Specializations.
+  template<>
+    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);
+
+    bind_textdomain_codeset(__s.c_str(),
+	__nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
+    return get_catalogs()._M_add(__s, __l);
+  }
+
+  template<>
+    void
+    messages<char>::do_close(catalog __c) const
+    { get_catalogs()._M_erase(__c); }
+
+  template<>
+    string
+    messages<char>::do_get(catalog __c, int, int,
+			   const string& __dfault) const
+    {
+      if (__c < 0 || __dfault.empty())
+	return __dfault;
+
+      const Catalog_info* __cat_info = get_catalogs()._M_get(__c);
+
+      if (!__cat_info)
+	return __dfault;
+
+      return get_glibc_msg(_M_c_locale_messages,
+			   __cat_info->_M_domain.c_str(),
+			   __dfault.c_str());
     }
 
 #ifdef _GLIBCXX_USE_WCHAR_T
+  template<>
+    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);
+
+    bind_textdomain_codeset(__s.c_str(),
+	__nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
+
+    return get_catalogs()._M_add(__s, __l);
+  }
+
+  template<>
+    void
+    messages<wchar_t>::do_close(catalog __c) const
+    { get_catalogs()._M_erase(__c); }
+
   template<>
     wstring
-    messages<wchar_t>::do_get(catalog, int, int, const wstring& __dfault) const
+    messages<wchar_t>::do_get(catalog __c, int, int,
+			      const wstring& __wdfault) const
     {
-# if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ > 2)
+      if (__c < 0 || __wdfault.empty())
-      __c_locale __old = __uselocale(_M_c_locale_messages);
+	return __wdfault;
-      char* __msg = gettext(_M_convert_to_char(__dfault));
+
-      __uselocale(__old);
+      const Catalog_info* __cat_info = get_catalogs()._M_get(__c);
-      return _M_convert_from_char(__msg);
+
-# else
+      if (!__cat_info)
-      char* __old = setlocale(LC_ALL, 0);
+	return __wdfault;
-      const size_t __len = strlen(__old) + 1;
+
-      char* __sav = new char[__len];
+      typedef codecvt<wchar_t, char, mbstate_t> __codecvt_t;
-      memcpy(__sav, __old, __len);
+      const __codecvt_t& __conv =
-      setlocale(LC_ALL, _M_name_messages);
+	use_facet<__codecvt_t>(__cat_info->_M_locale);
-      char* __msg = gettext(_M_convert_to_char(__dfault));
+
-      setlocale(LC_ALL, __sav);
+      const char* __translation;
-      delete [] __sav;
+      mbstate_t __state;
-      return _M_convert_from_char(__msg);
+      __builtin_memset(&__state, 0, sizeof(mbstate_t));
-# endif
+      {
+	const wchar_t* __wdfault_next;
+	size_t __mb_size = __wdfault.size() * __conv.max_length();;
+	char* __dfault =
+	  static_cast<char*>(__builtin_alloca(sizeof(char) * (__mb_size + 1)));
+	char* __dfault_next;
+	__conv.out(__state,
+		   __wdfault.data(), __wdfault.data() + __wdfault.size(),
+		   __wdfault_next,
+		   __dfault, __dfault + __mb_size, __dfault_next);
+
+	// 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);
+
+	// 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);
+      const char* __translation_next;
+      wchar_t* __wtranslation =
+	static_cast<wchar_t*>(__builtin_alloca(sizeof(wchar_t) * (__size + 1)));
+      wchar_t* __wtranslation_next;
+      __conv.in(__state, __translation, __translation + __size,
+		__translation_next,
+		__wtranslation, __wtranslation + __size,
+		__wtranslation_next);
+      return wstring(__wtranslation, __wtranslation_next);
     }
 #endif
 

libstdc++-v3/config/locale/gnu/messages_members.h

@@ -41,53 +41,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Non-virtual member functions.
   template<typename _CharT>
-     messages<_CharT>::messages(size_t __refs)
+    messages<_CharT>::messages(size_t __refs)
-     : facet(__refs), _M_c_locale_messages(_S_get_c_locale()), 
+    : facet(__refs), _M_c_locale_messages(_S_get_c_locale()),
-       _M_name_messages(_S_get_c_name())
+      _M_name_messages(_S_get_c_name())
-     { }
+    { }
 
   template<typename _CharT>
-     messages<_CharT>::messages(__c_locale __cloc, const char* __s, 
+    messages<_CharT>::messages(__c_locale __cloc, const char* __s,
-				size_t __refs) 
+			       size_t __refs)
-     : facet(__refs), _M_c_locale_messages(0), _M_name_messages(0)
+    : facet(__refs), _M_c_locale_messages(0), _M_name_messages(0)
-     {
+    {
-       if (__builtin_strcmp(__s, _S_get_c_name()) != 0)
+      if (__builtin_strcmp(__s, _S_get_c_name()) != 0)
-	 {
+	{
-	   const size_t __len = __builtin_strlen(__s) + 1;
+	  const size_t __len = __builtin_strlen(__s) + 1;
-	   char* __tmp = new char[__len];
+	  char* __tmp = new char[__len];
-	   __builtin_memcpy(__tmp, __s, __len);
+	  __builtin_memcpy(__tmp, __s, __len);
-	   _M_name_messages = __tmp;
+	  _M_name_messages = __tmp;
-	 }
+	}
-       else
+      else
-	 _M_name_messages = _S_get_c_name();
+	_M_name_messages = _S_get_c_name();
 
-       // Last to avoid leaking memory if new throws.
+      // Last to avoid leaking memory if new throws.
-       _M_c_locale_messages = _S_clone_c_locale(__cloc);
+      _M_c_locale_messages = _S_clone_c_locale(__cloc);
-     }
+    }
 
   template<typename _CharT>
-    typename messages<_CharT>::catalog 
+    typename messages<_CharT>::catalog
-    messages<_CharT>::open(const basic_string<char>& __s, const locale& __loc, 
+    messages<_CharT>::open(const basic_string<char>& __s, const locale& __loc,
 			   const char* __dir) const
-    { 
+    {
       bindtextdomain(__s.c_str(), __dir);
-      return this->do_open(__s, __loc); 
+      return this->do_open(__s, __loc);
     }
 
   // Virtual member functions.
   template<typename _CharT>
     messages<_CharT>::~messages()
-    { 
+    {
       if (_M_name_messages != _S_get_c_name())
 	delete [] _M_name_messages;
-      _S_destroy_c_locale(_M_c_locale_messages); 
+      _S_destroy_c_locale(_M_c_locale_messages);
     }
 
   template<typename _CharT>
-    typename messages<_CharT>::catalog 
+    typename messages<_CharT>::catalog
-    messages<_CharT>::do_open(const basic_string<char>& __s, 
+    messages<_CharT>::do_open(const basic_string<char>& __s,
 			      const locale&) const
-    { 
+    {
       // No error checking is done, assume the catalog exists and can
       // be used.
       textdomain(__s.c_str());
@@ -95,36 +95,57 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
   template<typename _CharT>
-    void    
+    void
-    messages<_CharT>::do_close(catalog) const 
+    messages<_CharT>::do_close(catalog) const
     { }
 
-   // messages_byname
+  // messages_byname
-   template<typename _CharT>
+  template<typename _CharT>
-     messages_byname<_CharT>::messages_byname(const char* __s, size_t __refs)
+    messages_byname<_CharT>::messages_byname(const char* __s, size_t __refs)
-     : messages<_CharT>(__refs) 
+    : messages<_CharT>(__refs)
-     { 
+    {
-       if (this->_M_name_messages != locale::facet::_S_get_c_name())
+      if (this->_M_name_messages != locale::facet::_S_get_c_name())
-	 {
+	{
-	   delete [] this->_M_name_messages;
+	  delete [] this->_M_name_messages;
-	   if (__builtin_strcmp(__s, locale::facet::_S_get_c_name()) != 0)
+	  if (__builtin_strcmp(__s, locale::facet::_S_get_c_name()) != 0)
-	     {
+	    {
-	       const size_t __len = __builtin_strlen(__s) + 1;
+	      const size_t __len = __builtin_strlen(__s) + 1;
-	       char* __tmp = new char[__len];
+	      char* __tmp = new char[__len];
-	       __builtin_memcpy(__tmp, __s, __len);
+	      __builtin_memcpy(__tmp, __s, __len);
-	       this->_M_name_messages = __tmp;
+	      this->_M_name_messages = __tmp;
-	     }
+	    }
-	   else
+	  else
-	     this->_M_name_messages = locale::facet::_S_get_c_name();
+	    this->_M_name_messages = locale::facet::_S_get_c_name();
-	 }
+	}
 
-       if (__builtin_strcmp(__s, "C") != 0
+      if (__builtin_strcmp(__s, "C") != 0
-	   && __builtin_strcmp(__s, "POSIX") != 0)
+	  && __builtin_strcmp(__s, "POSIX") != 0)
-	 {
+	{
-	   this->_S_destroy_c_locale(this->_M_c_locale_messages);
+	  this->_S_destroy_c_locale(this->_M_c_locale_messages);
-	   this->_S_create_c_locale(this->_M_c_locale_messages, __s); 
+	  this->_S_create_c_locale(this->_M_c_locale_messages, __s);
-	 }
+	}
-     }
+    }
+
+   //Specializations.
+  template<>
+    typename messages<char>::catalog
+    messages<char>::do_open(const basic_string<char>&,
+			    const locale&) const;
+
+  template<>
+    void
+    messages<char>::do_close(catalog) const;
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+  template<>
+    typename messages<wchar_t>::catalog
+    messages<wchar_t>::do_open(const basic_string<char>&,
+			       const locale&) const;
+
+  template<>
+    void
+    messages<wchar_t>::do_close(catalog) const;
+#endif
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace

libstdc++-v3/include/bits/codecvt.h

@@ -263,8 +263,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       do_max_length() const throw() = 0;
     };
 
-
-
   /**
    *  @brief  Primary class template codecvt.
    *  @ingroup locales
@@ -340,6 +338,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     class codecvt<char, char, mbstate_t>
     : public __codecvt_abstract_base<char, char, mbstate_t>
     {
+      friend class messages<char>;
+
     public:
       // Types:
       typedef char			intern_type;
@@ -398,6 +398,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     class codecvt<wchar_t, char, mbstate_t>
     : public __codecvt_abstract_base<wchar_t, char, mbstate_t>
     {
+      friend class messages<wchar_t>;
+
     public:
       // Types:
       typedef wchar_t			intern_type;

libstdc++-v3/testsuite/22_locale/messages/13631.cc

@@ -0,0 +1,99 @@
+// { dg-require-namedlocale "fr_FR" }
+
+// Copyright (C) 2014 Free Software Foundation
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <locale>
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  // This is defined through CXXFLAGS in scripts/testsuite_flags[.in].
+  const char* dir = LOCALEDIR;
+
+  std::locale l("fr_FR");
+
+  typedef std::messages<char> messages;
+
+  const messages &msgs_facet = std::use_facet<messages>(l);
+
+  messages::catalog msgs = msgs_facet.open("libstdc++", l, dir);
+  VERIFY( msgs >= 0 );
+
+  const char msgid[] = "please";
+  std::string translation1 = msgs_facet.get(msgs, 0, 0, msgid);
+
+  // Without a real translation this test doesn't mean anything:
+  VERIFY( translation1 != msgid );
+
+  // Opening another catalog was enough to show the problem, even a fake
+  // catalog.
+  messages::catalog fake_msgs = msgs_facet.open("fake", l);
+
+  std::string translation2 = msgs_facet.get(msgs, 0, 0, msgid);
+
+  // Close catalogs before doing the check to avoid leaks.
+  msgs_facet.close(fake_msgs);
+  msgs_facet.close(msgs);
+
+  VERIFY( translation1 == translation2 );
+}
+
+void test02()
+{
+  bool test __attribute__((unused)) = true;
+
+  // This is defined through CXXFLAGS in scripts/testsuite_flags[.in].
+  const char* dir = LOCALEDIR;
+
+  std::locale l("fr_FR");
+
+  typedef std::messages<wchar_t> messages;
+
+  const messages &msgs_facet = std::use_facet<messages>(l);
+
+  messages::catalog msgs = msgs_facet.open("libstdc++", l, dir);
+  VERIFY( msgs >= 0 );
+
+  const wchar_t msgid[] = L"please";
+  std::wstring translation1 = msgs_facet.get(msgs, 0, 0, msgid);
+
+  // Without a real translation this test doesn't mean anything:
+  VERIFY( !translation1.empty() );
+  VERIFY( translation1 != msgid );
+
+  // Opening another catalog was enough to show the problem, even a fake
+  // catalog.
+  messages::catalog fake_msgs = msgs_facet.open("fake", l);
+
+  std::wstring translation2 = msgs_facet.get(msgs, 0, 0, msgid);
+
+  // Close catalogs before doing the check to avoid leaks.
+  msgs_facet.close(fake_msgs);
+  msgs_facet.close(msgs);
+
+  VERIFY( translation1 == translation2 );
+}
+
+int main()
+{
+  test01();
+  test02();
+  return 0;
+}

libstdc++-v3/testsuite/22_locale/messages/members/char/2.cc

@@ -35,9 +35,8 @@ void test02()
   const char* dir = LOCALEDIR;
 
   // basic construction
-  locale loc_c = locale::classic();
   locale loc_fr = locale("fr_FR");
-  VERIFY( loc_c != loc_fr );
+  VERIFY( locale::classic() != loc_fr );
 
   // cache the messages facets
   const messages<char>& mssg_fr = use_facet<messages<char> >(loc_fr); 
@@ -47,7 +46,7 @@ void test02()
   // void close(catalog) const;
 
   // Check French (fr_FR) locale.
-  catalog cat_fr = mssg_fr.open("libstdc++", loc_c, dir);
+  catalog cat_fr = mssg_fr.open("libstdc++", loc_fr, dir);
   string s01 = mssg_fr.get(cat_fr, 0, 0, "please");
   string s02 = mssg_fr.get(cat_fr, 0, 0, "thank you");
   VERIFY ( s01 == "s'il vous plaît" );