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

Julien Fischer jfischer at opturion.com
Mon Apr 18 09:49:23 AEST 2016


Hi,

On Fri, 15 Apr 2016, Mark Brown wrote:

> This fixes a bug I hit recently, where some foreign code was not
> picking up an environment variable I had set using
> io.set_environment_var/4. The problem occurred after a GC freed the
> memory held by the string passed to putenv(), which doesn't make a
> copy on my platform (linux, glibc 2.21).
>
> In its current form the change affects C backends for which MR_WIN32
> is not defined. This is conservative: online documentation tells me it
> is not needed on OSX or FreeBSD either, but I can't easily test that.

> 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.
> +    %
> +:- mutable(putenv_strings, map(string, string), map.init, ground, [untrailed]).

I know that the most recent version of Mark's diff removed the mutable,
but I think it's worth mentioning: please try to avoid using mutables in
the standard library.  If something that depends on them ends up being
called by the library intialisation code all sorts of hard-to-debug
stuff can occur.

W.r.t to Mark's change: I think we also need to add

     :- pred have_set_environment_var is semidet.

so that programs can test whether the unerlying platfrom supports
modifying the environment.

Julien.


More information about the reviews mailing list