Refactor ExecuteAndWait to take StringRefs.

This simplifies some code which had StringRefs to begin with, and
makes other code more complicated which had const char* to begin
with.

In the end, I think this makes for a more idiomatic and platform
agnostic API.  Not all platforms launch process with null terminated
c-string arrays for the environment pointer and argv, but the api
was designed that way because it allowed easy pass-through for
posix-based platforms.  There's a little additional overhead now
since on posix based platforms we'll be takign StringRefs which
were constructed from null terminated strings and then copying
them to null terminate them again, but from a readability and
usability standpoint of the API user, I think this API signature
is strictly better.

llvm-svn: 334518

1 files changed, 28 insertions, 7 deletions

diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index be971555b82..d0abc3763e8 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -23,6 +23,7 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #if HAVE_SYS_STAT_H
 #include <sys/stat.h>
@@ -164,8 +165,18 @@ static void SetMemoryLimits(unsigned size) {
 
 }
 
-static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
-                    const char **Envp, ArrayRef<Optional<StringRef>> Redirects,
+static std::vector<const char *>
+toNullTerminatedCStringArray(ArrayRef<StringRef> Strings, StringSaver &Saver) {
+  std::vector<const char *> Result;
+  for (StringRef S : Strings)
+    Result.push_back(Saver.save(S).data());
+  Result.push_back(nullptr);
+  return Result;
+}
+
+static bool Execute(ProcessInfo &PI, StringRef Program,
+                    ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
+                    ArrayRef<Optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg) {
   if (!llvm::sys::fs::exists(Program)) {
     if (ErrMsg)
@@ -174,6 +185,18 @@ static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
     return false;
   }
 
+  BumpPtrAllocator Allocator;
+  StringSaver Saver(Allocator);
+  std::vector<const char *> ArgVector, EnvVector;
+  const char **Argv = nullptr;
+  const char **Envp = nullptr;
+  ArgVector = toNullTerminatedCStringArray(Args, Saver);
+  Argv = ArgVector.data();
+  if (Env) {
+    EnvVector = toNullTerminatedCStringArray(*Env, Saver);
+    Envp = EnvVector.data();
+  }
+
   // If this OS has posix_spawn and there is no memory limit being implied, use
   // posix_spawn.  It is more efficient than fork/exec.
 #ifdef HAVE_POSIX_SPAWN
@@ -227,7 +250,7 @@ static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
     // positive.
     pid_t PID = 0;
     int Err = posix_spawn(&PID, Program.str().c_str(), FileActions,
-                          /*attrp*/nullptr, const_cast<char **>(Args),
+                          /*attrp*/ nullptr, const_cast<char **>(Argv),
                           const_cast<char **>(Envp));
 
     if (FileActions)
@@ -280,12 +303,10 @@ static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
       // Execute!
       std::string PathStr = Program;
       if (Envp != nullptr)
-        execve(PathStr.c_str(),
-               const_cast<char **>(Args),
+        execve(PathStr.c_str(), const_cast<char **>(Argv),
                const_cast<char **>(Envp));
       else
-        execv(PathStr.c_str(),
-              const_cast<char **>(Args));
+        execv(PathStr.c_str(), const_cast<char **>(Argv));
       // If the execve() failed, we should exit. Follow Unix protocol and
       // return 127 if the executable was not found, and 126 otherwise.
       // Use _exit rather than exit so that atexit functions and static