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

Paul Bone paul at bone.id.au
Mon Apr 18 08:48:45 AEST 2016


On Fri, Apr 15, 2016 at 11:41:26PM +1000, Mark Brown wrote:
> Hi everyone,
> 
> 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.
> 
> For review by anyone.
> 
> Cheers,
> Mark

> 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]).

Would it not be easier just to duplicate the string in memory, allocating
the new copy with the system malloc() or GC_MALLOC_UNCOLLECTABLE()?

-- 
Paul Bone


More information about the reviews mailing list