stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <Travis.Vi...@roguewave.com>
Subject RE: svn commit: r644364 - in /stdcxx/trunk: src/locale_global.cpp tests/localization/22.locale.statics.mt.cpp
Date Thu, 03 Apr 2008 19:04:47 GMT
 

>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.
>

Mime
View raw message