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
--
