[m-rev.] for review: bugfix for io.set_environment_var/4

[Peter Wang]

On Fri, 15 Apr 2016 23:41:26 +1000, Mark Brown <mark at mercurylang.org> wrote:
> diff --git a/library/io.m b/library/io.m
> index 28fab1a..2c32e73 100644
> --- a/library/io.m
> +++ b/library/io.m
> @@ -10280,23 +10280,45 @@ command_line_argument(_, "") :-
>      end
>  ").
>  
> +    % Glibc requires that we keep a copy of strings we pass to putenv(), so
> +    % we store them here to prevent them being garbage collected. This map
> +    % is *not* used to retrieve values from the environment: a map is used
> +    % so that strings can be easily removed if a subsequent call sets a
> +    % new value for the environment variable.
> +    %

Hi Mark,

Judging by the man pages available online, newer versions of OSX and
FreeBSD confirm to POSIX specified behaviour, i.e. do not copy the
string.  It is not limited to glibc.

>      % io.putenv(VarString): If VarString is a string of the form "name=value",
>      % sets the environment variable name to the specified value. Fails if
>      % the operation does not work. This should only be called from io.setenv.
>      %
> -:- impure pred io.putenv(string::in) is semidet.
> +    % Returns 'yes' in the second argument if the caller is required to keep
> +    % a copy of VarString (i.e., prevent it being garbage collected or freed).
> +    %
> +:- impure pred io.putenv(string::in, bool::out) is semidet.
>  
>  :- pragma foreign_proc("C",
> -    io.putenv(VarAndValue::in),
> +    io.putenv(VarAndValue::in, Keep::out),
>      [will_not_call_mercury, not_thread_safe, tabled_for_io,
>          does_not_affect_liveness, no_sharing],
>  "
>  #ifdef MR_WIN32
> +    Keep = MR_NO;
>      SUCCESS_INDICATOR = (_wputenv(ML_utf8_to_wide(VarAndValue)) == 0);
>  #else
> +    Keep = MR_YES;
>      SUCCESS_INDICATOR = (putenv(VarAndValue) == 0);
>  #endif
>  ").

Since putenv is utterly broken, I suggest using setenv instead
(and unsetenv if required).

Windows has SetEnvironmentVariable.  I have not been able to find
documentation stating whether it duplicates its input, which hopefully
means it does.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686206(v=vs.85).aspx

Peter