Mon, 01/16/2012 - 00:46
Working on #0022898 I had a little look on the source code regarding locale handling and string to real conversion.
Why is there a CStringToReal function in class OSD and in class OSD_Real2String with the same signature? The functions are also used in a very few cases. A "grep -r StringToReal *" in the src-folder gives only 23 matches.
In contrast to that a "grep -r atof *" leads to 1030 matches. The problem with atof is, that there is no possibility to know if everything works as expected. strtod has this possibility, but "grep -r strtod *" gives only 15 matches (including the use in the both mentioned OSD classes).
Is there a reason to have this different implementations? Otherwise I would suggest to change the code to only use one. One of the OSD-functions would have the advantage to be able to detect wrong conversions (due to different locale settings).
Also I am wondering why there is class OSD_Localizer, that seems to be used only once (in OpenGl_GraphicDriver_Export.cxx). All other changing of locales seems to be done by setlocale(). Could this be changed, so there is only one used?
Tue, 01/17/2012 - 15:21
I agree we need to revise how locales are managed within OCCT. In most cases, I believe, plain C locale must be used. At least this is a case for IGES and STEP import / export. Thus there we need to set C locale unconditionally as you did (naturally taking care of restoring it back, and considering possible multithreading).
Note that most of places where atof() is used are in DRAW commands, and here also C locale will likely be most appropriate (for compatibility of test scripts). However, we might need to have possibility to change locale in DRAW to test how OCCT works with other locales.
I suggest you register an issue for this in Mantis.
Wed, 01/18/2012 - 03:28
Is there a plan what to use for locale handling, setlocale() or OSD_Localizer, in the future? It would be nice to have the same solution everywhere.
Wed, 01/18/2012 - 04:29
I believe OSD_Localizer is the right tool to be used everywhere (unless some reason appears for not using it). It is not yet used widely just because it appeared recently and until now no one took care of updating the whole OCCT code for that.
Fri, 01/20/2012 - 15:26
setlocale() is not thread-safe, using C++ locales is IMHO a better idea.
Fri, 01/20/2012 - 18:23
The point in using OSD_Localizer everywhere throughout the code is just that we can then more easily improve this implementation to employ thread-safe variant of setlocale() or make other improvements as needed (and they are needed!). Frankly speaking I am not very well aware of how locales work, this is just a general consideration.
Fri, 01/20/2012 - 19:16
Just FYI.
Due to using locks (instead of atomic operations), Visual C++ (at least until VS10 included) offers no scalability when using locales concurrently (e.g. via streams). Moreover performance may fall dramatically when used in highly contended scenarios.
Please see here (the issue I submitted to Microsoft in 2009):
http://social.msdn.microsoft.com/Forums/en/vcgeneral/thread/b049dbda-c11...
http://connect.microsoft.com/VisualStudio/feedback/details/492561/stl-st...
Mon, 01/30/2012 - 03:02
I created a ticket for a new OSD_Localizer implementation and added a implementation with uselocale() and _configthreadlocale():
http://tracker.dev.opencascade.org/view.php?id=22933
Perhaps this can be a starting point.
Thu, 02/07/2013 - 11:37
Hello,
Last week a fix for #0022898 has been integrated to master branch, everyone interested in this topic is welcome to check how this works.
This fix implements new functions like Atof() and Sprintf() working always in C locale which are used throughout OCCT code to do string conversions involving reals. Localizer class (now called Standard_CLocaleSentry) is still provided for use in context where change of global locale is the only way to manage conversion (currently used for call to Gl2Ps). See the issue for more details.
Andrey