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.

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();

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

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

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
 

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