Eric Lemings wrote:
[...]
Please peer review the following patch at your earliest convenience.
-----
$ svn diff
Index: src/setlocale.h
===================================================================
--- src/setlocale.h (revision 644426)
+++ src/setlocale.h (working copy)
@@ -58,6 +58,9 @@
return _C_name;
}
+ static const char* __rw_curlocale (int);
+ static bool __rw_newlocale (const char*, int);
+
private:
__rw_setlocale (const __rw_setlocale&); // not defined
Index: src/setlocale.cpp
===================================================================
--- src/setlocale.cpp (revision 644426)
+++ src/setlocale.cpp (working copy)
@@ -80,11 +80,8 @@
__rw_setlocale::__rw_setlocale (const char *locname, int cat, int
nothrow)
: _C_name (0), _C_guard (1), _C_cat (cat)
{
- // acquire global per-process lock
- __rw_setlocale_mutex._C_acquire ();
-
// retrieve previous locale name and check if it is already set
- const char* const curname = ::setlocale (cat, 0);
+ const char* const curname = __rw_curlocale (cat);
if (curname) {
@@ -101,14 +98,13 @@
::memcpy (_C_name, curname, size);
This doesn't seem safe: some other thread might have called
setlocale by now and invalidated our curname. The ctor needs
to hold the lock until it's finished copying the name of the
locale returned by setlocale().
IMO, the least invasive change to __rw_setlocale is to expose
the __rw_setlocale_mutex object (currently defined static in
setlocale.cpp) so that it can be used with the _RWSTD_MT_GUARD()
macro. That way we won't have to change any existing
__rw_setlocale code. The fix to locale::global() will then
be to add
_RWSTD_MT_GUARD (__rw_setlocale::_C_mutex);
at the top of the function.
Martin
- if (::setlocale (cat, locname)) {
+ if (__rw_newlocale (locname, cat)) {
// a NULL name is only used when we want to use the object
// as a retriever for the name of the current locale;
// no need for a lock then
if (!locname) {
_C_guard = 0;
- __rw_setlocale_mutex._C_release ();
}
return;
@@ -122,7 +118,6 @@
// bad locales OR bad locale names OR bad locale categories
_C_guard = 0;
- __rw_setlocale_mutex._C_release ();
// failure in setting the locale
_RWSTD_REQUIRES (nothrow, (_RWSTD_ERROR_LOCALE_BAD_NAME,
@@ -131,6 +126,25 @@
}
+/*static*/ const char*
+__rw_setlocale::__rw_curlocale (int cat)
+{
+ __rw_setlocale_mutex._C_acquire ();
+ const char* const curname = ::setlocale (cat, 0);
+ __rw_setlocale_mutex._C_release ();
+ return curname;
+}
+
+/*static*/ bool
+__rw_setlocale::__rw_newlocale (const char* locname, int cat)
+{
+ __rw_setlocale_mutex._C_acquire ();
+ bool changed = (0 != ::setlocale (cat, locname));
+ __rw_setlocale_mutex._C_release ();
+ return changed;
+}
+
+
// dtor restores the C library locale that
// was in effect when the object was constructed
__rw_setlocale::~__rw_setlocale ()
Index: src/locale_global.cpp
===================================================================
--- src/locale_global.cpp (revision 644426)
+++ src/locale_global.cpp (working copy)
@@ -54,8 +54,7 @@
// assumes all locale names (i.e., including those of combined
// locales) are in a format understandable by setlocale()
- const _RW::__rw_setlocale clocale (rhs.name().c_str(),
_RWSTD_LC_ALL);
- _RWSTD_UNUSED(clocale);
+ _RW::__rw_setlocale::__rw_newlocale (rhs.name().c_str(),
_RWSTD_LC_ALL);
// FIXME:
// handle unnamed locales (i.e., locales containing
non-standard
-----
Thanks,
Brad.
