cbernardo / libIGES

Implementation of the IGESv5.3 specification
http://cbernardo.github.io/libIGES
GNU Lesser General Public License v2.1
58 stars 18 forks source link

C Locale changed and not restored #16

Open ramcdona opened 5 years ago

ramcdona commented 5 years ago

I haven't had time to fully investigate, but users of my program with a custom locale set report writing corrupted files after exporting IGES files. My program uses the C locale -- until libIGES changes it to something matching their system settings.

IMHO, libIGES should either not set the locale -- or it should restore the locale after it is done doing whatever it needs to set the locale for. At a minimum, the docs should make it very clear that libIGES may change the locale.

cbernardo commented 5 years ago

I'll try to find time to look into this. The idea was that IGES set the locale to C and restored the locale when it was done loading/saving; that was necessary since IGES requires an ASCII character set (7 bit characters); I'm not even sure if using 8-bit characters is safe.  The problem with C (and I don't know if C11 fixed this issue) is that locale changes affect all threads.  I think C++11 has a mechanism for locale changes which only affect a specific stream, and libIGES has C++ at its core, but 8 years later I still haven't got much of an idea of how widely implemented that feature is. When I last checked  2 years ago it was impossible to use the stream locale feature safely on multiple platforms or with multiple compilers. Cirilo

  From: Rob McDonald <notifications@github.com>

To: cbernardo/libIGES libIGES@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Sent: Thursday, January 24, 2019 5:14 AM Subject: [cbernardo/libIGES] C Locale changed and not restored (#16)

I haven't had time to fully investigate, but users of my program with a custom locale set report writing corrupted files after exporting IGES files. My program uses the C locale -- until libIGES changes it to something matching their system settings.IMHO, libIGES should either not set the locale -- or it should restore the locale after it is done doing whatever it needs to set the locale for. At a minimum, the docs should make it very clear that libIGES may change the locale.— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

ramcdona commented 5 years ago

Thanks much,

My program does not set the locale -- so it stays as the C locale. I suspect when libIGES finishes writing, it re-sets to some locale specified by the system or environment variables (not to a stored variable set to the locale before libIGES changed it).

I could force the C locale before writing anything, but it was very unexpected to have it change.

cbernardo commented 5 years ago

I altered the behavior of libiges slightly. The previous behavior was to set the locale to the default environment; now the locale is set to whatever the setting was when libiges was invoked to read/write a file. So if the locale was set to "C" when a load/save function was called, it should remain as "C". Let me know if this fixes the problem.

ramcdona commented 5 years ago

Thanks much,

That sounds like it should fix the issue. Unfortunately, I haven't been able to duplicate the problem -- I'll have to give it a go and see if my users notice an improvement.

It certainly sounds like improved behavior.

Best,

Rob

On Sun, Mar 10, 2019 at 10:28 PM cbernardo notifications@github.com wrote:

I altered the behavior of libiges slightly. The previous behavior was to set the locale to the default environment; now the locale is set to whatever the setting was when libiges was invoked to read/write a file. So if the locale was set to "C" when a load/save function was called, it should remain as "C". Let me know if this fixes the problem.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cbernardo/libIGES/issues/16#issuecomment-471408994, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRJwDcSQ39C_1EsDpkOT_osPTxaTDkVks5vVel5gaJpZM4aQJ50 .