Re: setting user environment variables

[Mike Miller]

On Mon, Mar 16, 2015 at 19:13:51 -0700, Rik wrote:
> A little code review help is in order.  Normally I wouldn't touch a user's
> environment, but since we've moved to a dispatch program that forks to a
> child I don't think this is so critical anymore as we are only affecting
> the child which vanishes at exit anyways.

I don't think there's really a difference in how the program executes,
environment changes should have the same effect either way.

> The issue is that the locale of the user is entering into Octave and
> causing problems.  Matlab, and Octave followed, unilaterally sets the
> LC_NUMERIC and LC_TIME variables to the 'C' locale.  This used to work, but
> apparently it no longer does as there are now reports on savannah of
> segfaults when these are not overriden (http://savannah.gnu.org/bugs/?44469).
> 
> The proposed patch, which was tested and works, is below.  Any objections? 
> 
> * octave.cc (octave_initialize_interpreter): Use octave_env::putenv to set
> LC_NUMERIC and LC_TIME variables to 'C' as Matlab does.
> 
> diff -r fc6c87e254bf -r bd06a3c3ef9d libinterp/octave.cc
> --- a/libinterp/octave.cc    Sun Mar 15 17:03:16 2015 +0000
> +++ b/libinterp/octave.cc    Mon Mar 16 11:51:35 2015 -0700
> @@ -727,6 +727,8 @@ octave_initialize_interpreter (int argc,
>    // Matlab uses "C" locale for LC_NUMERIC class regardless of local setting
>    setlocale (LC_NUMERIC, "C");
>    setlocale (LC_TIME, "C");
> +  octave_env::putenv ("LC_NUMERIC", "C");
> +  octave_env::putenv ("LC_TIME", "C");

I agree that something like this may be necessary. The way I read the
specification of setlocale [1], a future call to the function at any
time during program execution may reset locale settings from the values
that are resident in the environment. This may be what's causing the
reported error.

I understand your patch has been tested to work, but something like the
following makes more sense to me (override the values in the
environment, then call setlocale to initialize all the settings):

diff --git a/libinterp/octave.cc b/libinterp/octave.cc
--- a/libinterp/octave.cc
+++ b/libinterp/octave.cc
@@ -725,8 +725,9 @@ void
 octave_initialize_interpreter (int argc, char **argv, int embedded)
 {
   // Matlab uses "C" locale for LC_NUMERIC class regardless of local setting
-  setlocale (LC_NUMERIC, "C");
-  setlocale (LC_TIME, "C");
+  octave_env::putenv ("LC_NUMERIC", "C");
+  octave_env::putenv ("LC_TIME", "C");
+  setlocale (LC_ALL, 0);
 
   octave_embedded = embedded;
 

But both may be effectively the same thing.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/setlocale.html

-- 
mike