On Thu, Oct 1, 2009 at 2:03 AM, Rick McGuire <[email protected]> wrote:
> Ok, I've taken a look at the code, and I don't understand how this
> works either, and it certainly could lead to some crashes or memory
> problems because that pointer in the einvironment is still pointing to
> memory it should not be. It would appear the appropriate fix would be
> to call putenv() to set the target variable to a null string value.
> That will ensure the new value is set properly.
This whole area seems to be a mess to me. The
SetEnvironmentVariable() sys_process_export(), and
restoreEnvironment() functions all have this code at the start:
if (!putflag)
{ /* first change in the */
/* environment ? */
/* copy all entries to dynamic memory */
/*for all entries in the env */
for (;*Environment != NULL;Environment++)
{
length = strlen(*Environment)+1; /* get the size of the string */
/* and alloc space for it */
Env_Var_String = (char *)malloc(length);
memcpy(Env_Var_String,*Environment,length);/* copy the string */
putenv(Env_Var_String); /* and make it part of env */
}
putflag = 1; /* prevent do it again */
}
>From a comment I found in some old Gnu code:
/* On Solaris, HP-UX and IRIX there is no function to clear an environment
variable. So we look for the variable in the environ table and delete it
by setting the entry to NULL. This can clearly cause some memory leaks
but free cannot be used on this context as not all strings in the environ
have been allocated using malloc. To avoid this memory leak another
method can be used. It consists in forcing the reallocation of all the
strings in the environ table using malloc on the first call on the
functions related to environment variable management. The disavantage
is that if a program makes a direct call to getenv the return string
may be deallocated at some point. */
I think this malloc'ng a copy of every environment string and putting
it back into environ was an attempt to support the clearing of an
environment variable on platforms that didn't have unsetenv() and
avoid the memory leak by using the method referenced above.
AIX didn't have unsetenv() in 5.1 but does in 5.3
All 3 of the functions have the:
if (del) /* if there was an old entry */
{
free(del); /* free it */
}
code. Which seems inherently unsafe, but the reason for it is to free
the memory malloc'd in the original copying of all the environment
strings. (Or malloc'd in subsequent replacment strings.) All the man
pages for putenv() that I have seen say the string passed in becomes
part of the environment and should not be altered or freed. So, we
don't really know what putenv() is going to do.
All of these 3 areas could be rewritten to use unsetenv(), getenv(),
and setenv() which would be cleaner and safer.
unsetenv() is on Linux and Mac. For AIX, I just found a reference
that unsetenv() is on AIX 5.2, which is the earliest we say we
support. For Solaris we say we support 2.8, which doesn't have
unsetenv(), but I think 2.10 does. (Although I couldn't find anything
definite.)
I have this patch for ValueFunction.cpp
Index: interpreter/platform/unix/ValueFunction.cpp
===================================================================
--- interpreter/platform/unix/ValueFunction.cpp (revision 5228)
+++ interpreter/platform/unix/ValueFunction.cpp (working copy)
@@ -103,11 +103,16 @@
del = *Environment; /* remember it for deletion */
}
- if (Value != (RexxString *) TheNilObject)
+ if (Value == (RexxString *) TheNilObject)
{
+ sprintf(Env_Var_String, "%s=%s",Name->getStringData(),"");
+ }
+ else
+ {
sprintf(Env_Var_String,
"%s=%s",Name->getStringData(),Value->getStringData());
- putenv(Env_Var_String);
}
+ putenv(Env_Var_String);
+
if(del) /* if there was a old one */
free(del); /* free it */
return 0; /* return success */
Which tests okay on Linux.
Since we are not going to change the release code to fix value() on
Mac, maybe we should open a bug so we don't forget about it and
rewrite those sections? (I'll do the rewrite.) Or, I can just apply
the patch. Or ... ?
--
Mark Miesfeld
------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Oorexx-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/oorexx-devel
