[Cmake-commits] CMake branch, next, updated. v2.8.8-2720-g1c80fc1

Brad King brad.king at kitware.com
Thu Apr 26 15:21:24 EDT 2012


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "CMake".

The branch, next has been updated
       via  1c80fc164a17d8e0ee53a9c0a3fb3278bd610db7 (commit)
       via  b67d0527e2bc416eea9766aacd4049dbaee9f160 (commit)
       via  b10c5cbb878af541f4ffa35a9e287c31743c6e97 (commit)
      from  5d477bdb30934561b3a8eadd35c525903f7aceab (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1c80fc164a17d8e0ee53a9c0a3fb3278bd610db7
commit 1c80fc164a17d8e0ee53a9c0a3fb3278bd610db7
Merge: 5d477bd b67d052
Author:     Brad King <brad.king at kitware.com>
AuthorDate: Thu Apr 26 15:21:20 2012 -0400
Commit:     CMake Topic Stage <kwrobot at kitware.com>
CommitDate: Thu Apr 26 15:21:20 2012 -0400

    Merge topic 'kwsys-environ-cleanup' into next
    
    b67d052 KWSys: Fix SystemTools environment memory handling (#13156)
    b10c5cb CTest: Simplify environment save/restore


http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=b67d0527e2bc416eea9766aacd4049dbaee9f160
commit b67d0527e2bc416eea9766aacd4049dbaee9f160
Author:     Brad King <brad.king at kitware.com>
AuthorDate: Thu Apr 26 10:36:53 2012 -0400
Commit:     Brad King <brad.king at kitware.com>
CommitDate: Thu Apr 26 13:58:39 2012 -0400

    KWSys: Fix SystemTools environment memory handling (#13156)
    
    The SystemTools::PutEnv function tries to provide the "putenv" API
    without leaking memory.  However, the kwsysDeletingCharVector singleton
    frees memory that may still be referenced by the environment table,
    having been placed there by putenv.  If any static destruction or
    processing by an external tool happens after the singleton is destroyed
    and accesses the environment it will read invalid memory.
    
    Replace use of putenv with setenv/unsetenv when available.  The latter
    manage internal copies of the values passed instead of referencing the
    original memory.  When setenv/unsetenv are not available use putenv with
    a singleton that removes its values from the environment before freeing
    their memory.  This requires an "unputenv" implementation.  On at least
    some platforms it must be written in terms of "putenv" because other
    APIs are not available and direct modification of the "environ" global
    is not safe (e.g. on Windows there is interaction with "wenviron").
    Fortunately either putenv("A=") or putenv("A") will remove "A" from the
    environment on these platforms.  On other platforms fall back to direct
    manipulation of "environ".
    
    Also add UnPutEnv to the API and add a test for the behavior of both.

diff --git a/Source/kwsys/CMakeLists.txt b/Source/kwsys/CMakeLists.txt
index 0ef4e28..f51712c 100644
--- a/Source/kwsys/CMakeLists.txt
+++ b/Source/kwsys/CMakeLists.txt
@@ -553,11 +553,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.
diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
index 4d83293..7c85fd4 100644
--- a/Source/kwsys/SystemTools.cxx
+++ b/Source/kwsys/SystemTools.cxx
@@ -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,38 +381,224 @@ 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;
+}
 
-kwsysDeletingCharVector::~kwsysDeletingCharVector()
+#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)
 {
-#ifndef KWSYS_DO_NOT_CLEAN_PUTENV
-  for(kwsys_stl::vector<char*>::iterator i = this->begin();
-      i != this->end(); ++i)
+  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)
     {
-    delete []*i;
+    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::PutEnv(const char* value)
+
+bool SystemTools::UnPutEnv(const char* env)
 {
-  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;
+  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 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;
+      }
+    }
+};
+
+class kwsysEnv: public kwsys_stl::set<const char*, kwsysEnvCompare>
+{
+  class Free
+  {
+    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
+
+//----------------------------------------------------------------------------
+
 const char* SystemTools::GetExecutableExtension()
 {
 #if defined(_WIN32) || defined(__CYGWIN__) || defined(__VMS)
diff --git a/Source/kwsys/SystemTools.hxx.in b/Source/kwsys/SystemTools.hxx.in
index 04f1978..5171125 100644
--- a/Source/kwsys/SystemTools.hxx.in
+++ b/Source/kwsys/SystemTools.hxx.in
@@ -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
diff --git a/Source/kwsys/kwsysPlatformTestsCXX.cxx b/Source/kwsys/kwsysPlatformTestsCXX.cxx
index 903be9b..16124d3 100644
--- a/Source/kwsys/kwsysPlatformTestsCXX.cxx
+++ b/Source/kwsys/kwsysPlatformTestsCXX.cxx
@@ -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.  */
 
diff --git a/Source/kwsys/testSystemTools.cxx b/Source/kwsys/testSystemTools.cxx
index c0e74af..3ac0cb3 100644
--- a/Source/kwsys/testSystemTools.cxx
+++ b/Source/kwsys/testSystemTools.cxx
@@ -328,6 +328,58 @@ bool CheckStringOperations()
 }
 
 //----------------------------------------------------------------------------
+
+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*[])
 {
   bool res = true;
@@ -356,5 +408,7 @@ int testSystemTools(int, char*[])
 
   res &= CheckStringOperations();
 
+  res &= CheckEnvironmentOperations();
+
   return res ? 0 : 1;
 }
diff --git a/bootstrap b/bootstrap
index 0069754..5b431fd 100755
--- a/bootstrap
+++ b/bootstrap
@@ -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
@@ -1050,6 +1052,33 @@ 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
   KWSYS_STL_HAVE_STD=1
@@ -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"`

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=b10c5cbb878af541f4ffa35a9e287c31743c6e97
commit b10c5cbb878af541f4ffa35a9e287c31743c6e97
Author:     Brad King <brad.king at kitware.com>
AuthorDate: Tue Apr 24 16:24:17 2012 -0400
Commit:     Brad King <brad.king at kitware.com>
CommitDate: Tue Apr 24 16:56:14 2012 -0400

    CTest: Simplify environment save/restore
    
    Replace use of AppendEnv/RestoreEnv pairs with instances of
    SaveRestoreEnvironment.  Simplify the signature of AppendEnv and use it
    in place of similar loops elsewhere.  Move the RestoreEnv implementation
    inside the SaveRestoreEnvironment destructor which is the only place
    left that calls it.

diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index 81f18b0..c3de5dc 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -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();
diff --git a/Source/CTest/cmCTestScriptHandler.cxx b/Source/CTest/cmCTestScriptHandler.cxx
index 5841b8d..d3ab2ef 100644
--- a/Source/CTest/cmCTestScriptHandler.cxx
+++ b/Source/CTest/cmCTestScriptHandler.cxx
@@ -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
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index 3f7fdc7..4aff64b 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -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;
 }
 
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index 1376a48..548fa74 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -1630,33 +1630,28 @@ 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());
-      }
+    cmSystemTools::PutEnv(eit->c_str());
     }
-
-  return origEnv;
 }
 
 //----------------------------------------------------------------------
-void cmSystemTools::RestoreEnv(const std::vector<std::string>& env)
+cmSystemTools::SaveRestoreEnvironment::SaveRestoreEnvironment()
 {
-  std::vector<std::string>::const_iterator eit;
+  this->Env = cmSystemTools::GetEnvironmentVariables();
+}
 
+//----------------------------------------------------------------------
+cmSystemTools::SaveRestoreEnvironment::~SaveRestoreEnvironment()
+{
   // First clear everything in the current environment:
-  //
   std::vector<std::string> currentEnv = GetEnvironmentVariables();
-  for (eit = currentEnv.begin(); eit!= currentEnv.end(); ++eit)
+  for(std::vector<std::string>::const_iterator
+        eit = currentEnv.begin(); eit != currentEnv.end(); ++eit)
     {
     std::string var(*eit);
 
@@ -1666,27 +1661,11 @@ void cmSystemTools::RestoreEnv(const std::vector<std::string>& env)
       var = var.substr(0, pos);
       }
 
-    UnsetEnv(var.c_str());
+    cmSystemTools::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::SaveRestoreEnvironment::SaveRestoreEnvironment()
-{
-  this->Env = cmSystemTools::GetEnvironmentVariables();
-}
-
-//----------------------------------------------------------------------
-cmSystemTools::SaveRestoreEnvironment::~SaveRestoreEnvironment()
-{
-  cmSystemTools::RestoreEnv(this->Env);
+  cmSystemTools::AppendEnv(this->Env);
 }
 #endif
 
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 5f21de2..69673c9 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -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

-----------------------------------------------------------------------

Summary of changes:
 Source/CTest/cmCTestRunTest.cxx        |    2 +-
 Source/CTest/cmCTestScriptHandler.cxx  |    6 +-
 Source/cmCTest.cxx                     |   23 +--
 Source/cmSystemTools.cxx               |   49 ++-----
 Source/cmSystemTools.h                 |   12 +--
 Source/kwsys/CMakeLists.txt            |   13 ++-
 Source/kwsys/SystemTools.cxx           |  236 +++++++++++++++++++++++++++++---
 Source/kwsys/SystemTools.hxx.in        |    6 +-
 Source/kwsys/kwsysPlatformTestsCXX.cxx |   26 ++++
 Source/kwsys/testSystemTools.cxx       |   54 +++++++
 bootstrap                              |   37 +++++-
 11 files changed, 370 insertions(+), 94 deletions(-)


hooks/post-receive
-- 
CMake


More information about the Cmake-commits mailing list