On Tue, Mar 17, 2015 at 12:32 AM, Ian Romanick <i...@freedesktop.org> wrote: > On 03/15/2015 12:05 PM, Erik Faye-Lund wrote: >> _mesa_strtod and _mesa_strtof are only used from the GLSL compiler, > > It's also used in the ARB_vertex_program / ARB_fragment_program > assembler in src/prog.
Oh, right. Thanks for pointing that out. >> so the locale doesn't need to be initialized before the first context >> gets initialized. So let's use explicit initialization from the >> one-time init code instead of depending on a C++ compiler to initialize >> at image-load time. > > This is fairly close to the way Chia-I originally had it: > > http://lists.freedesktop.org/archives/mesa-dev/2014-April/058215.html > > Some discussion of alternate methods started: > > http://lists.freedesktop.org/archives/mesa-dev/2014-May/058861.html Thanks for pointing me at this discussion, very useful. > I'm a little concerned that having the initialization in Mesa and the > function accessible to both Mesa and Gallium that we may set ourselves > up for problems later. Doesn't really sound like a ground-shattering risk to me. But perhaps adding an assert verifying that initialization was done could offset that risk? > It also occurs to me that the neither the old code nor the new code ever > call freelocale. In OpenGL ES 2.0 and OpenGL 4.1, you have glReleaseShaderCompiler which is intended for this kind of work. But I'm not sure a single leak of a locale is really worth the implementation-effort. > I think that's easier to fix with the static object > method (using a destructor). > > I guess I'm kind of ambivalent about the change. Yeah, especially initialization having to be done in three different locations causes me to start losing some confidence that this is a good idea. >> Signed-off-by: Erik Faye-Lund <kusmab...@gmail.com> >> --- >> >> Because of the recent discussion on libc++ and Mesa, I thought I'd >> have a look into what parts of mesa depended on libc++, and I spotted >> this file. >> >> In this case, it was rather trivial to port the code to plain C, making >> it dead obvious that it doesn't depend on libc++. I'm not proposing all >> C++ gets this treatment, but in this case it seems like a pretty >> straight-forward way to make it obvious that this code does not depend >> on libc++. >> >> src/mesa/main/context.c | 3 +++ >> src/util/Makefile.sources | 2 +- >> src/util/{strtod.cpp => strtod.c} | 14 ++++++++------ >> src/util/strtod.h | 3 +++ >> 4 files changed, 15 insertions(+), 7 deletions(-) >> rename src/util/{strtod.cpp => strtod.c} (89%) >> >> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c >> index 22c2341..de6a016 100644 >> --- a/src/mesa/main/context.c >> +++ b/src/mesa/main/context.c >> @@ -119,6 +119,7 @@ >> #include "shared.h" >> #include "shaderobj.h" >> #include "util/simple_list.h" >> +#include "util/strtod.h" >> #include "state.h" >> #include "stencil.h" >> #include "texcompress_s3tc.h" >> @@ -398,6 +399,8 @@ one_time_init( struct gl_context *ctx ) >> assert( sizeof(GLint) == 4 ); >> assert( sizeof(GLuint) == 4 ); >> >> + _mesa_locale_init(); >> + >> _mesa_one_time_init_extension_overrides(); >> >> _mesa_get_cpu_features(); >> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources >> index 560ea83..f930790 100644 >> --- a/src/util/Makefile.sources >> +++ b/src/util/Makefile.sources >> @@ -17,7 +17,7 @@ MESA_UTIL_FILES := \ >> set.c \ >> set.h \ >> simple_list.h \ >> - strtod.cpp \ >> + strtod.c \ >> strtod.h \ >> texcompress_rgtc_tmp.h \ >> u_atomic.h >> diff --git a/src/util/strtod.cpp b/src/util/strtod.c >> similarity index 89% >> rename from src/util/strtod.cpp >> rename to src/util/strtod.c >> index 2b4dd98..a4a60e0 100644 >> --- a/src/util/strtod.cpp >> +++ b/src/util/strtod.c >> @@ -30,18 +30,20 @@ >> #include <locale.h> >> #ifdef HAVE_XLOCALE_H >> #include <xlocale.h> >> +static locale_t loc; >> #endif >> #endif >> >> #include "strtod.h" >> >> >> +void >> +_mesa_locale_init(void) >> +{ >> #if defined(_GNU_SOURCE) && defined(HAVE_XLOCALE_H) >> -static struct locale_initializer { >> - locale_initializer() { loc = newlocale(LC_CTYPE_MASK, "C", NULL); } >> - locale_t loc; >> -} loc_init; >> + loc = newlocale(LC_CTYPE_MASK, "C", NULL); >> #endif >> +} >> >> /** >> * Wrapper around strtod which uses the "C" locale so the decimal >> @@ -51,7 +53,7 @@ double >> _mesa_strtod(const char *s, char **end) >> { >> #if defined(_GNU_SOURCE) && defined(HAVE_XLOCALE_H) >> - return strtod_l(s, end, loc_init.loc); >> + return strtod_l(s, end, loc); >> #else >> return strtod(s, end); >> #endif >> @@ -66,7 +68,7 @@ float >> _mesa_strtof(const char *s, char **end) >> { >> #if defined(_GNU_SOURCE) && defined(HAVE_XLOCALE_H) >> - return strtof_l(s, end, loc_init.loc); >> + return strtof_l(s, end, loc); >> #elif defined(HAVE_STRTOF) >> return strtof(s, end); >> #else >> diff --git a/src/util/strtod.h b/src/util/strtod.h >> index 02c25dd..b7e2beb 100644 >> --- a/src/util/strtod.h >> +++ b/src/util/strtod.h >> @@ -31,6 +31,9 @@ >> extern "C" { >> #endif >> >> +extern void >> +_mesa_locale_init(void); >> + >> extern double >> _mesa_strtod(const char *s, char **end); >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev