Merge topic 'kwsys-environ-cleanup'

e48796b KWSys: Fix SystemTools environment memory handling (#13156)
b10c5cb CTest: Simplify environment save/restore
This commit is contained in:
David Cole 2012-05-01 14:09:12 -04:00 committed by CMake Topic Stage
commit c75f404065
11 changed files with 379 additions and 103 deletions

View File

@ -671,7 +671,7 @@ bool cmCTestRunTest::ForkProcess(double testTimeOut, bool explicitTimeout,
if (environment && environment->size()>0)
{
cmSystemTools::AppendEnv(environment);
cmSystemTools::AppendEnv(*environment);
}
return this->TestProcess->StartProcess();

View File

@ -643,11 +643,7 @@ int cmCTestScriptHandler::RunCurrentScript()
{
std::vector<std::string> envArgs;
cmSystemTools::ExpandListArgument(this->CTestEnv.c_str(),envArgs);
// for each variable/argument do a putenv
for (unsigned i = 0; i < envArgs.size(); ++i)
{
cmSystemTools::PutEnv(envArgs[i].c_str());
}
cmSystemTools::AppendEnv(envArgs);
}
// now that we have done most of the error checking finally run the

View File

@ -48,7 +48,7 @@
#include <float.h>
#include <ctype.h>
#include <memory> // auto_ptr
#include <cmsys/auto_ptr.hxx>
#include <cm_zlib.h>
#include <cmsys/Base64.h>
@ -509,7 +509,7 @@ int cmCTest::Initialize(const char* binary_dir, cmCTestStartCommand* command)
cmake cm;
cmGlobalGenerator gg;
gg.SetCMakeInstance(&cm);
std::auto_ptr<cmLocalGenerator> lg(gg.CreateLocalGenerator());
cmsys::auto_ptr<cmLocalGenerator> lg(gg.CreateLocalGenerator());
cmMakefile *mf = lg->GetMakefile();
if ( !this->ReadCustomConfigurationFileTree(this->BinaryDir.c_str(), mf) )
{
@ -1277,7 +1277,6 @@ int cmCTest::RunTest(std::vector<const char*> argv,
std::ostream* log, double testTimeOut,
std::vector<std::string>* environment)
{
std::vector<std::string> origEnv;
bool modifyEnv = (environment && environment->size()>0);
// determine how much time we have
@ -1334,9 +1333,11 @@ int cmCTest::RunTest(std::vector<const char*> argv,
}
std::string oldpath = cmSystemTools::GetCurrentWorkingDirectory();
cmsys::auto_ptr<cmSystemTools::SaveRestoreEnvironment> saveEnv;
if (modifyEnv)
{
origEnv = cmSystemTools::AppendEnv(environment);
saveEnv.reset(new cmSystemTools::SaveRestoreEnvironment);
cmSystemTools::AppendEnv(*environment);
}
*retVal = inst.Run(args, output);
@ -1351,11 +1352,6 @@ int cmCTest::RunTest(std::vector<const char*> argv,
"Internal cmCTest object used to run test." << std::endl
<< *output << std::endl);
if (modifyEnv)
{
cmSystemTools::RestoreEnv(origEnv);
}
return cmsysProcess_State_Exited;
}
std::vector<char> tempOutput;
@ -1364,9 +1360,11 @@ int cmCTest::RunTest(std::vector<const char*> argv,
*output = "";
}
cmsys::auto_ptr<cmSystemTools::SaveRestoreEnvironment> saveEnv;
if (modifyEnv)
{
origEnv = cmSystemTools::AppendEnv(environment);
saveEnv.reset(new cmSystemTools::SaveRestoreEnvironment);
cmSystemTools::AppendEnv(*environment);
}
cmsysProcess* cp = cmsysProcess_New();
@ -1436,11 +1434,6 @@ int cmCTest::RunTest(std::vector<const char*> argv,
}
cmsysProcess_Delete(cp);
if (modifyEnv)
{
cmSystemTools::RestoreEnv(origEnv);
}
return result;
}

View File

@ -1633,50 +1633,12 @@ std::vector<std::string> cmSystemTools::GetEnvironmentVariables()
}
//----------------------------------------------------------------------
std::vector<std::string> cmSystemTools::AppendEnv(
std::vector<std::string>* env)
void cmSystemTools::AppendEnv(std::vector<std::string> const& env)
{
std::vector<std::string> origEnv = GetEnvironmentVariables();
if (env && env->size()>0)
for(std::vector<std::string>::const_iterator eit = env.begin();
eit != env.end(); ++eit)
{
std::vector<std::string>::const_iterator eit;
for (eit = env->begin(); eit!= env->end(); ++eit)
{
PutEnv(eit->c_str());
}
}
return origEnv;
}
//----------------------------------------------------------------------
void cmSystemTools::RestoreEnv(const std::vector<std::string>& env)
{
std::vector<std::string>::const_iterator eit;
// First clear everything in the current environment:
//
std::vector<std::string> currentEnv = GetEnvironmentVariables();
for (eit = currentEnv.begin(); eit!= currentEnv.end(); ++eit)
{
std::string var(*eit);
std::string::size_type pos = var.find("=");
if (pos != std::string::npos)
{
var = var.substr(0, pos);
}
UnsetEnv(var.c_str());
}
// Then put back each entry from the original environment:
//
for (eit = env.begin(); eit!= env.end(); ++eit)
{
PutEnv(eit->c_str());
cmSystemTools::PutEnv(eit->c_str());
}
}
@ -1689,7 +1651,24 @@ cmSystemTools::SaveRestoreEnvironment::SaveRestoreEnvironment()
//----------------------------------------------------------------------
cmSystemTools::SaveRestoreEnvironment::~SaveRestoreEnvironment()
{
cmSystemTools::RestoreEnv(this->Env);
// First clear everything in the current environment:
std::vector<std::string> currentEnv = GetEnvironmentVariables();
for(std::vector<std::string>::const_iterator
eit = currentEnv.begin(); eit != currentEnv.end(); ++eit)
{
std::string var(*eit);
std::string::size_type pos = var.find("=");
if (pos != std::string::npos)
{
var = var.substr(0, pos);
}
cmSystemTools::UnsetEnv(var.c_str());
}
// Then put back each entry from the original environment:
cmSystemTools::AppendEnv(this->Env);
}
#endif

View File

@ -371,16 +371,8 @@ public:
/** Get the list of all environment variables */
static std::vector<std::string> GetEnvironmentVariables();
/** Append multiple variables to the current environment.
Return the original environment, as it was before the
append. */
static std::vector<std::string> AppendEnv(
std::vector<std::string>* env);
/** Restore the full environment to "env" - use after
AppendEnv to put the environment back to the way it
was. */
static void RestoreEnv(const std::vector<std::string>& env);
/** Append multiple variables to the current environment. */
static void AppendEnv(std::vector<std::string> const& env);
/** Helper class to save and restore the environment.
Instantiate this class as an automatic variable on

View File

@ -552,11 +552,16 @@ SET_SOURCE_FILES_PROPERTIES(ProcessUNIX.c System.c PROPERTIES
COMPILE_FLAGS "-DKWSYS_C_HAS_PTRDIFF_T=${KWSYS_C_HAS_PTRDIFF_T} -DKWSYS_C_HAS_SSIZE_T=${KWSYS_C_HAS_SSIZE_T}"
)
IF(KWSYS_DO_NOT_CLEAN_PUTENV)
# Disable cleanup of putenv memory for issues with GCOV.
IF(KWSYS_USE_SystemTools)
KWSYS_PLATFORM_CXX_TEST(KWSYS_CXX_HAS_SETENV
"Checking whether CXX compiler has setenv" DIRECT)
KWSYS_PLATFORM_CXX_TEST(KWSYS_CXX_HAS_UNSETENV
"Checking whether CXX compiler has unsetenv" DIRECT)
KWSYS_PLATFORM_CXX_TEST(KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H
"Checking whether CXX compiler has environ in stdlib.h" DIRECT)
SET_SOURCE_FILES_PROPERTIES(SystemTools.cxx PROPERTIES
COMPILE_FLAGS -DKWSYS_DO_NOT_CLEAN_PUTENV=1)
ENDIF(KWSYS_DO_NOT_CLEAN_PUTENV)
COMPILE_FLAGS "-DKWSYS_CXX_HAS_SETENV=${KWSYS_CXX_HAS_SETENV} -DKWSYS_CXX_HAS_UNSETENV=${KWSYS_CXX_HAS_UNSETENV} -DKWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H=${KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H}")
ENDIF()
#-----------------------------------------------------------------------------
# Choose a directory for the generated headers.

View File

@ -25,6 +25,8 @@
#include KWSYS_HEADER(ios/fstream)
#include KWSYS_HEADER(ios/sstream)
#include KWSYS_HEADER(stl/set)
// Work-around CMake dependency scanning limitation. This must
// duplicate the above list of headers.
#if 0
@ -78,6 +80,14 @@
# undef _WIN32
#endif
#if !KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H
# if defined(_WIN32)
extern __declspec(dllimport) char **environ;
# else
extern char **environ;
# endif
#endif
#ifdef __CYGWIN__
extern "C" void cygwin_conv_to_win32_path(const char *path, char *win32_path);
#endif
@ -371,37 +381,223 @@ bool SystemTools::GetEnv(const char* key, kwsys_stl::string& result)
}
}
#ifdef __INTEL_COMPILER
#pragma warning disable 444
//----------------------------------------------------------------------------
#if defined(__CYGWIN__) || defined(__GLIBC__)
# define KWSYS_PUTENV_NAME /* putenv("A") removes A. */
#elif defined(_WIN32)
# define KWSYS_PUTENV_EMPTY /* putenv("A=") removes A. */
#endif
class kwsysDeletingCharVector : public kwsys_stl::vector<char*>
#if KWSYS_CXX_HAS_UNSETENV
/* unsetenv("A") removes A from the environment.
On older platforms it returns void instead of int. */
static int kwsysUnPutEnv(const char* env)
{
public:
~kwsysDeletingCharVector();
if(const char* eq = strchr(env, '='))
{
std::string name(env, eq-env);
unsetenv(name.c_str());
}
else
{
unsetenv(env);
}
return 0;
}
#elif defined(KWSYS_PUTENV_EMPTY) || defined(KWSYS_PUTENV_NAME)
/* putenv("A=") or putenv("A") removes A from the environment. */
static int kwsysUnPutEnv(const char* env)
{
int err = 0;
const char* eq = strchr(env, '=');
size_t const len = eq? (size_t)(eq-env) : strlen(env);
# ifdef KWSYS_PUTENV_EMPTY
size_t const sz = len + 2;
# else
size_t const sz = len + 1;
# endif
char local_buf[256];
char* buf = sz > sizeof(local_buf) ? (char*)malloc(sz) : local_buf;
if(!buf)
{
return -1;
}
strncpy(buf, env, len);
# ifdef KWSYS_PUTENV_EMPTY
buf[len] = '=';
buf[len+1] = 0;
if(putenv(buf) < 0)
{
err = errno;
}
# else
buf[len] = 0;
if(putenv(buf) < 0 && errno != EINVAL)
{
err = errno;
}
# endif
if(buf != local_buf)
{
free(buf);
}
if(err)
{
errno = err;
return -1;
}
return 0;
}
#else
/* Manipulate the "environ" global directly. */
static int kwsysUnPutEnv(const char* env)
{
const char* eq = strchr(env, '=');
size_t const len = eq? (size_t)(eq-env) : strlen(env);
int in = 0;
int out = 0;
while(environ[in])
{
if(strlen(environ[in]) > len &&
environ[in][len] == '=' &&
strncmp(env, environ[in], len) == 0)
{
++in;
}
else
{
environ[out++] = environ[in++];
}
}
while(out < in)
{
environ[out++] = 0;
}
return 0;
}
#endif
//----------------------------------------------------------------------------
#if KWSYS_CXX_HAS_SETENV
/* setenv("A", "B", 1) will set A=B in the environment and makes its
own copies of the strings. */
bool SystemTools::PutEnv(const char* env)
{
if(const char* eq = strchr(env, '='))
{
std::string name(env, eq-env);
return setenv(name.c_str(), eq+1, 1) == 0;
}
else
{
return kwsysUnPutEnv(env) == 0;
}
}
bool SystemTools::UnPutEnv(const char* env)
{
return kwsysUnPutEnv(env) == 0;
}
#else
/* putenv("A=B") will set A=B in the environment. Most putenv implementations
put their argument directly in the environment. They never free the memory
on program exit. Keep an active set of pointers to memory we allocate and
pass to putenv, one per environment key. At program exit remove any
environment values that may still reference memory we allocated. Then free
the memory. This will not affect any environment values we never set. */
# ifdef __INTEL_COMPILER
# pragma warning disable 444 /* base has non-virtual destructor */
# endif
/* Order by environment key only (VAR from VAR=VALUE). */
struct kwsysEnvCompare
{
bool operator() (const char* l, const char* r) const
{
const char* leq = strchr(l, '=');
const char* req = strchr(r, '=');
size_t llen = leq? (leq-l) : strlen(l);
size_t rlen = req? (req-r) : strlen(r);
if(llen == rlen)
{
return strncmp(l,r,llen) < 0;
}
else
{
return strcmp(l,r) < 0;
}
}
};
kwsysDeletingCharVector::~kwsysDeletingCharVector()
class kwsysEnv: public kwsys_stl::set<const char*, kwsysEnvCompare>
{
#ifndef KWSYS_DO_NOT_CLEAN_PUTENV
for(kwsys_stl::vector<char*>::iterator i = this->begin();
i != this->end(); ++i)
class Free
{
delete []*i;
const char* Env;
public:
Free(const char* env): Env(env) {}
~Free() { free(const_cast<char*>(this->Env)); }
};
public:
typedef kwsys_stl::set<const char*, kwsysEnvCompare> derived;
~kwsysEnv()
{
for(derived::iterator i = this->begin(); i != this->end(); ++i)
{
kwsysUnPutEnv(*i);
free(const_cast<char*>(*i));
}
}
const char* Release(const char* env)
{
const char* old = 0;
derived::iterator i = this->find(env);
if(i != this->end())
{
old = *i;
this->erase(i);
}
return old;
}
bool Put(const char* env)
{
Free oldEnv(this->Release(env));
static_cast<void>(oldEnv);
char* newEnv = strdup(env);
this->insert(newEnv);
return putenv(newEnv) == 0;
}
bool UnPut(const char* env)
{
Free oldEnv(this->Release(env));
static_cast<void>(oldEnv);
return kwsysUnPutEnv(env) == 0;
}
};
static kwsysEnv kwsysEnvInstance;
bool SystemTools::PutEnv(const char* env)
{
return kwsysEnvInstance.Put(env);
}
bool SystemTools::UnPutEnv(const char* env)
{
return kwsysEnvInstance.UnPut(env);
}
#endif
}
bool SystemTools::PutEnv(const char* value)
{
static kwsysDeletingCharVector localEnvironment;
char* envVar = new char[strlen(value)+1];
strcpy(envVar, value);
int ret = putenv(envVar);
// save the pointer in the static vector so that it can
// be deleted on exit
localEnvironment.push_back(envVar);
return ret == 0;
}
//----------------------------------------------------------------------------
const char* SystemTools::GetExecutableExtension()
{

View File

@ -749,7 +749,11 @@ public:
/** Put a string into the environment
of the form var=value */
static bool PutEnv(const char* value);
static bool PutEnv(const char* env);
/** Remove a string from the environment.
Input is of the form "var" or "var=value" (value is ignored). */
static bool UnPutEnv(const char* env);
/**
* Get current working directory CWD

View File

@ -393,6 +393,32 @@ int main(int, char **argv)
}
#endif
#ifdef TEST_KWSYS_CXX_HAS_SETENV
#include <stdlib.h>
int main()
{
return setenv("A", "B", 1);
}
#endif
#ifdef TEST_KWSYS_CXX_HAS_UNSETENV
#include <stdlib.h>
int main()
{
unsetenv("A");
return 0;
}
#endif
#ifdef TEST_KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H
#include <stdlib.h>
int main()
{
char* e = environ[0];
return e? 0:1;
}
#endif
#ifdef TEST_KWSYS_CXX_TYPE_INFO
/* Collect fundamental type information and save it to a CMake script. */

View File

@ -327,6 +327,58 @@ bool CheckStringOperations()
return res;
}
//----------------------------------------------------------------------------
bool CheckPutEnv(const char* env, const char* name, const char* value)
{
if(!kwsys::SystemTools::PutEnv(env))
{
kwsys_ios::cerr << "PutEnv(\"" << env
<< "\") failed!" << kwsys_ios::endl;
return false;
}
const char* v = kwsys::SystemTools::GetEnv(name);
v = v? v : "(null)";
if(strcmp(v, value) != 0)
{
kwsys_ios::cerr << "GetEnv(\"" << name << "\") returned \""
<< v << "\", not \"" << value << "\"!" << kwsys_ios::endl;
return false;
}
return true;
}
bool CheckUnPutEnv(const char* env, const char* name)
{
if(!kwsys::SystemTools::UnPutEnv(env))
{
kwsys_ios::cerr << "UnPutEnv(\"" << env << "\") failed!"
<< kwsys_ios::endl;
return false;
}
if(const char* v = kwsys::SystemTools::GetEnv(name))
{
kwsys_ios::cerr << "GetEnv(\"" << name << "\") returned \""
<< v << "\", not (null)!" << kwsys_ios::endl;
return false;
}
return true;
}
bool CheckEnvironmentOperations()
{
bool res = true;
res &= CheckPutEnv("A=B", "A", "B");
res &= CheckPutEnv("B=C", "B", "C");
res &= CheckPutEnv("C=D", "C", "D");
res &= CheckPutEnv("D=E", "D", "E");
res &= CheckUnPutEnv("A", "A");
res &= CheckUnPutEnv("B=", "B");
res &= CheckUnPutEnv("C=D", "C");
/* Leave "D=E" in environment so a memory checker can test for leaks. */
return res;
}
//----------------------------------------------------------------------------
int testSystemTools(int, char*[])
{
@ -356,5 +408,7 @@ int testSystemTools(int, char*[])
res &= CheckStringOperations();
res &= CheckEnvironmentOperations();
return res ? 0 : 1;
}

View File

@ -395,7 +395,6 @@ cmake_kwsys_config_replace_string ()
echo "${APPEND}" > "${OUTFILE}${_tmp}"
cat "${INFILE}" |
sed "/./ {s/\@KWSYS_NAMESPACE\@/cmsys/g;
s/@KWSYS_DO_NOT_CLEAN_PUTENV@/0/g;
s/@KWSYS_BUILD_SHARED@/${KWSYS_BUILD_SHARED}/g;
s/@KWSYS_LFS_AVAILABLE@/${KWSYS_LFS_AVAILABLE}/g;
s/@KWSYS_LFS_REQUESTED@/${KWSYS_LFS_REQUESTED}/g;
@ -1039,6 +1038,9 @@ KWSYS_STL_HAS_ALLOCATOR_NONTEMPLATE=0
KWSYS_STL_HAS_ALLOCATOR_REBIND=0
KWSYS_STL_HAS_ALLOCATOR_MAX_SIZE_ARGUMENT=0
KWSYS_STL_HAS_ALLOCATOR_OBJECTS=0
KWSYS_CXX_HAS_SETENV=0
KWSYS_CXX_HAS_UNSETENV=0
KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H=0
KWSYS_CXX_HAS_CSTDDEF=0
KWSYS_CXX_HAS_NULL_TEMPLATE_ARGS=0
KWSYS_CXX_HAS_MEMBER_TEMPLATES=0
@ -1049,6 +1051,33 @@ KWSYS_CXX_HAS_ARGUMENT_DEPENDENT_LOOKUP=0
KWSYS_STL_STRING_HAVE_ISTREAM=1
KWSYS_STL_STRING_HAVE_OSTREAM=1
if cmake_try_run "${cmake_cxx_compiler}" \
"${cmake_cxx_flags} -DTEST_KWSYS_CXX_HAS_SETENV" \
"${cmake_source_dir}/Source/kwsys/kwsysPlatformTestsCXX.cxx" >> cmake_bootstrap.log 2>&1; then
KWSYS_CXX_HAS_SETENV=1
echo "${cmake_cxx_compiler} has setenv"
else
echo "${cmake_cxx_compiler} does not have setenv"
fi
if cmake_try_run "${cmake_cxx_compiler}" \
"${cmake_cxx_flags} -DTEST_KWSYS_CXX_HAS_UNSETENV" \
"${cmake_source_dir}/Source/kwsys/kwsysPlatformTestsCXX.cxx" >> cmake_bootstrap.log 2>&1; then
KWSYS_CXX_HAS_UNSETENV=1
echo "${cmake_cxx_compiler} has unsetenv"
else
echo "${cmake_cxx_compiler} does not have unsetenv"
fi
if cmake_try_run "${cmake_cxx_compiler}" \
"${cmake_cxx_flags} -DTEST_KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H" \
"${cmake_source_dir}/Source/kwsys/kwsysPlatformTestsCXX.cxx" >> cmake_bootstrap.log 2>&1; then
KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H=1
echo "${cmake_cxx_compiler} has environ in stdlib.h"
else
echo "${cmake_cxx_compiler} does not have environ in stdlib.h"
fi
if cmake_try_run "${cmake_cxx_compiler}" \
"${cmake_cxx_flags} -DTEST_KWSYS_STL_HAVE_STD" \
"${cmake_source_dir}/Source/kwsys/kwsysPlatformTestsCXX.cxx" >> cmake_bootstrap.log 2>&1; then
@ -1365,7 +1394,7 @@ cmake_replace_string "${cmake_source_dir}/Source/kwsys/kwsys_stl.hxx.in" \
cmake_replace_string "${cmake_bootstrap_dir}/cmsys/stl/stl.hxx_a" \
"${cmake_bootstrap_dir}/cmsys/stl/stl.hxx_b" KWSYS_NAMESPACE cmsys
for a in string vector map algorithm; do
for a in string vector set map algorithm; do
cmake_replace_string "${cmake_bootstrap_dir}/cmsys/stl/stl.hxx_b" \
"${cmake_bootstrap_dir}/cmsys/stl/${a}" KWSYS_STL_HEADER ${a}
done
@ -1396,6 +1425,7 @@ if [ "x${cmake_cxx_flags}" != "x" ]; then
fi
cmake_c_flags_String="-DKWSYS_STRING_C"
cmake_cxx_flags_SystemTools="-DKWSYS_CXX_HAS_SETENV=${KWSYS_CXX_HAS_SETENV} -DKWSYS_CXX_HAS_UNSETENV=${KWSYS_CXX_HAS_UNSETENV} -DKWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H=${KWSYS_CXX_HAS_ENVIRON_IN_STDLIB_H}"
cmake_c_flags="${cmake_c_flags}-I`cmake_escape \"${cmake_bootstrap_dir}\"` -I`cmake_escape \"${cmake_source_dir}/Source\"` \
-I`cmake_escape \"${cmake_bootstrap_dir}\"`"
cmake_cxx_flags="${cmake_cxx_flags} -I`cmake_escape \"${cmake_bootstrap_dir}\"` -I`cmake_escape \"${cmake_source_dir}/Source\"` \
@ -1421,8 +1451,9 @@ for a in ${KWSYS_C_SOURCES} ${KWSYS_C_MINGW_SOURCES}; do
done
for a in ${KWSYS_CXX_SOURCES}; do
src=`cmake_escape "${cmake_source_dir}/Source/kwsys/${a}.cxx"`
src_flags=`eval echo \\${cmake_cxx_flags_\${a}}`
echo "${a}.o : ${src} ${dep}" >> "${cmake_bootstrap_dir}/Makefile"
echo " ${cmake_cxx_compiler} ${cmake_cxx_flags} -DKWSYS_NAMESPACE=cmsys -c ${src} -o ${a}.o" >> "${cmake_bootstrap_dir}/Makefile"
echo " ${cmake_cxx_compiler} ${cmake_cxx_flags} -DKWSYS_NAMESPACE=cmsys ${src_flags} -c ${src} -o ${a}.o" >> "${cmake_bootstrap_dir}/Makefile"
done
if ${cmake_system_mingw}; then
src=`cmake_escape "${cmake_bootstrap_dir}/cmsysProcessFwd9xEnc.c"`