[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