So here is another proposal with all your remarks implemented.
2014-12-02 François Dumont <fdum...@gcc.gnu.org>
DR 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.
Tested under Linux x86_64.
Ok to commit ?
François
On 02/12/2014 11:55, Jonathan Wakely wrote:
If some user has got code that uses messages<signed char> and they
provide a definition for messages<signed char>::do_get() their code
will break if do_open and do_close disappear. (Realistically I doubt
anyone is doing that though, it may not even work.)
I restored default implementation, it does no harm.
This is not exception safe. You can use auto_ptr to fix it:
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
Good catch.
Index: config/locale/gnu/messages_members.cc
===================================================================
--- config/locale/gnu/messages_members.cc (revision 218285)
+++ config/locale/gnu/messages_members.cc (working copy)
@@ -31,20 +31,123 @@
#include <locale>
#include <bits/c++locale_internal.h>
-namespace std _GLIBCXX_VISIBILITY(default)
+#include <limits>
+#include <algorithm>
+#include <vector>
+
+#include <backward/auto_ptr.h>
+#include <ext/concurrence.h>
+
+namespace
{
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
+ using namespace std;
- // Specializations.
- template<>
- string
- messages<char>::do_get(catalog, int, int, const string& __dfault) const
+ 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);
- const char* __msg = const_cast<const char*>(gettext(__dfault.c_str()));
+ std::__c_locale __old = __uselocale(__locale_messages);
+ 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;
@@ -51,35 +154,131 @@
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)
- __c_locale __old = __uselocale(_M_c_locale_messages);
- char* __msg = gettext(_M_convert_to_char(__dfault));
- __uselocale(__old);
- return _M_convert_from_char(__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);
- char* __msg = gettext(_M_convert_to_char(__dfault));
- setlocale(LC_ALL, __sav);
- delete [] __sav;
- return _M_convert_from_char(__msg);
-# endif
+ if (__c < 0 || __wdfault.empty())
+ return __wdfault;
+
+ const Catalog_info* __cat_info = get_catalogs()._M_get(__c);
+
+ if (!__cat_info)
+ return __wdfault;
+
+ typedef codecvt<wchar_t, char, mbstate_t> __codecvt_t;
+ const __codecvt_t& __conv =
+ use_facet<__codecvt_t>(__cat_info->_M_locale);
+
+ const char* __translation;
+ mbstate_t __state;
+ __builtin_memset(&__state, 0, sizeof(mbstate_t));
+ {
+ 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
_GLIBCXX_END_NAMESPACE_VERSION
Index: config/locale/gnu/messages_members.h
===================================================================
--- config/locale/gnu/messages_members.h (revision 218285)
+++ config/locale/gnu/messages_members.h (working copy)
@@ -126,5 +126,26 @@
}
}
+ //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
Index: include/bits/codecvt.h
===================================================================
--- include/bits/codecvt.h (revision 218285)
+++ include/bits/codecvt.h (working copy)
@@ -263,8 +263,6 @@
do_max_length() const throw() = 0;
};
-
-
/**
* @brief Primary class template codecvt.
* @ingroup locales
@@ -340,6 +338,8 @@
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 @@
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;
Index: testsuite/22_locale/messages/13631.cc
===================================================================
--- testsuite/22_locale/messages/13631.cc (revision 0)
+++ testsuite/22_locale/messages/13631.cc (working copy)
@@ -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;
+}
Index: testsuite/22_locale/messages/members/char/2.cc
===================================================================
--- testsuite/22_locale/messages/members/char/2.cc (revision 218285)
+++ testsuite/22_locale/messages/members/char/2.cc (working copy)
@@ -35,9 +35,8 @@
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 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" );
