apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: apr_env_set use of putenv
Date Mon, 29 Mar 2010 13:16:38 GMT
On 29 Mar 2010, at 2:38 PM, Dan Poirier wrote:

> If a platform doesn't have setenv(), then apr_env_set() uses putenv()
> with memory allocated from whatever pool it was given.  Putenv()  
> keeps a
> reference to that memory in the environment.  If that pool is ever
> cleaned up, then the environment ends up with a pointer to
> who-knows-what.
>
> The doc string for apr_env_set says:
>
> * @param pool where to allocate temporary storage from
>
> which implies a short-lived pool is fine to use here, when clearly  
> it's
> not.
>
> This seems like a bug waiting to hit some unsuspecting user of APR.  I
> think it might be a good idea to strdup() a copy of the value before
> passing it to putenv(), even though I don't see a good way to ever
> recover that memory.  E.g.
>
> Index: env.c
> ===================================================================
> --- env.c	(revision 24753)
> +++ env.c	(working copy)
> @@ -105,7 +105,7 @@
>     memcpy(p, value, vlen);
>     p[vlen] = '\0';
>
> -    if (0 > putenv(env))
> +    if (0 > putenv(strdup(env)))
>         return APR_ENOMEM;
>     return APR_SUCCESS;
>
> Thoughts?

I think registering a proper cleanup to remove the environment  
variable on pool cleanup is the way to go, possibly with the addition  
of apr_env_setn() that sets the environment without a corresponding  
cleanup (ie current behaviour).

This follows the pattern of apr_table and friends.

Regards,
Graham
--


Mime
View raw message