>Eric Lemings wrote:
>
>Please peer review the following patch at your earliest convenience.
>
Brad,
I don't think that this patch is going to work. The unnecessary changes
to the __rw_setlocale ctor and dtor will cause new failures.
The __rw_setlocale type is supposed to hold the lock for its lifetime to
prevent other __rw_setlocale instances from being created and changing
the locale. Your change prevents concurrent calls to setlocale() through
__rw_setlocale() instances, but it doesn't prevent multiple
__rw_setlocale instances from existing simultaneously.
Travis
>-----
>$ 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);
>
>- 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.
>
