stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Lemings" <Eric.Lemi...@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 18:06:58 GMT
 

> -----Original Message-----
> From: Eric Lemings 
> Sent: Thursday, April 03, 2008 11:12 AM
> To: 'dev@stdcxx.apache.org'
> Subject: RE: svn commit: r644364 - in /stdcxx/trunk: 
> src/locale_global.cpp tests/localization/22.locale.statics.mt.cpp
> 
>  
> 
> > -----Original Message-----
> > From: Martin Sebor [mailto:sebor@roguewave.com] 
> > Sent: Thursday, April 03, 2008 11:06 AM
> > To: dev@stdcxx.apache.org
> > Subject: Re: svn commit: r644364 - in /stdcxx/trunk: 
> > src/locale_global.cpp tests/localization/22.locale.statics.mt.cpp
> > 
> > Travis Vitek wrote:
> > >  
> > > 
> > >> -----Original Message-----
> > >> From: Martin Sebor [mailto:sebor@roguewave.com] 
> > >> Sent: Thursday, April 03, 2008 8:55 AM
> > >> To: dev@stdcxx.apache.org
> > >> Subject: Re: svn commit: r644364 - in /stdcxx/trunk: 
> > >> src/locale_global.cpp tests/localization/22.locale.statics.mt.cpp
> > >>
> > >> elemings@apache.org wrote:
> > >>> Author: elemings
> > >>> Date: Thu Apr  3 08:32:56 2008
> > >>> New Revision: 644364
> > >>>
> > >>> URL: http://svn.apache.org/viewvc?rev=644364&view=rev
> > >>> Log:
> > >>> 2008-04-03  Eric Lemings  <eric.lemings@roguewave.com>
> > >>>
> > >>> 	STDCXX-811
> > >>> 	* src/locale_global.cpp (std::locale::global): 
> Replace call to
> > >>> 	non-reentrant setlocale() C function with reentrant
> > >>> 	__rw_setlocale object.
> > >> I don't think that's correct. The object's dtor restores
> > >> the original locale. We need a mutex around the call to
> > >> setlocale. Look for/at the _RWSTD_STATIC_GUARD() and
> > >> _RWSTD_CLASS_GUARD() macros.
> > >>
> > > 
> > > Right. It seems that for this to be mt-safe with respect to other
> > > library code that calls setlocale(), we would need to lock 
> > the same lock
> > > that is used by _RW::__rw_setlocale. If don't use the same 
> > lock it would
> > > be possible for the std::locale::global() call to change 
> the locale
> > > while some other library code is reading locale specific 
> data under
> > > protection of an active _RW::__rw_setlocale instance.
> > 
> > Good point. That probably means extending the __rw_setlocale
> > interface to release the lock without restoring the original
> > locale name.
> 
> I'll whip up a patch to src/setlocale.[h/cpp] and post it for
> review before submitting the next change.

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);

-        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