Fix some memory leaks

3 files changed, 27 insertions, 36 deletions

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 3250b04eb9..7e61f968a6 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -414,19 +414,6 @@ static void commonChildInit(Pipe & logPipe)
     close(fdDevNull);
 }
 
-
-/* Convert a string list to an array of char pointers.  Careful: the
-   string list should outlive the array. */
-const char * * strings2CharPtrs(const Strings & ss)
-{
-    const char * * arr = new const char * [ss.size() + 1];
-    const char * * p = arr;
-    foreach (Strings::const_iterator, i, ss) *p++ = i->c_str();
-    *p = 0;
-    return arr;
-}
-
-
 /* Restore default handling of SIGPIPE, otherwise some programs will
    randomly say "Broken pipe". */
 static void restoreSIGPIPE()
@@ -2159,11 +2146,7 @@ void DerivationGoal::runChild()
         Strings envStrs;
         foreach (Environment::const_iterator, i, env)
             envStrs.push_back(rewriteHashes(i->first + "=" + i->second, rewritesToTmp));
-        const char * * envArr = strings2CharPtrs(envStrs);
-
-        Path program = drv.builder.c_str();
-        std::vector<const char *> args; /* careful with c_str()! */
-        string user; /* must be here for its c_str()! */
+        auto envArr = stringsToCharPtrs(envStrs);
 
         /* If we are running in `build-users' mode, then switch to the
            user we allocated above.  Make sure that we drop all root
@@ -2189,16 +2172,12 @@ void DerivationGoal::runChild()
         }
 
         /* Fill in the arguments. */
+        Strings args;
         string builderBasename = baseNameOf(drv.builder);
-        args.push_back(builderBasename.c_str());
-        foreach (Strings::iterator, i, drv.args) {
-            auto re = rewriteHashes(*i, rewritesToTmp);
-            auto cstr = new char[re.length()+1];
-            std::strcpy(cstr, re.c_str());
-
-            args.push_back(cstr);
-        }
-        args.push_back(0);
+        args.push_back(builderBasename);
+        foreach (Strings::iterator, i, drv.args)
+            args.push_back(rewriteHashes(*i, rewritesToTmp));
+        auto argArr = stringsToCharPtrs(args);
 
         restoreSIGPIPE();
 
@@ -2206,7 +2185,7 @@ void DerivationGoal::runChild()
         writeFull(STDERR_FILENO, "\n");
 
         /* Execute the program.  This should not return. */
-        execve(program.c_str(), (char * *) &args[0], (char * *) envArr);
+        execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) &envArr[0]);
 
         throw SysError(format("executing `%1%'") % drv.builder);
 
@@ -2836,7 +2815,7 @@ void SubstitutionGoal::tryToRun()
     args.push_back("--substitute");
     args.push_back(storePath);
     args.push_back(destPath);
-    const char * * argArr = strings2CharPtrs(args);
+    auto argArr = stringsToCharPtrs(args);
 
     /* Fork the substitute program. */
     pid = startProcess([&]() {
@@ -2846,7 +2825,7 @@ void SubstitutionGoal::tryToRun()
         if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
             throw SysError("cannot dup output pipe into stdout");
 
-        execv(sub.c_str(), (char * *) argArr);
+        execv(sub.c_str(), (char * *) &argArr[0]);
 
         throw SysError(format("executing `%1%'") % sub);
     });
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 4d6090d66d..7998664ed0 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -893,16 +893,19 @@ pid_t startProcess(std::function<void()> fun,
 }
 
 
+std::vector<const char *> stringsToCharPtrs(const Strings & ss)
+{
+    std::vector<const char *> res;
+    for (auto & s : ss) res.push_back(s.c_str());
+    res.push_back(0);
+    return res;
+}
+
+
 string runProgram(Path program, bool searchPath, const Strings & args)
 {
     checkInterrupt();
 
-    std::vector<const char *> cargs; /* careful with c_str()! */
-    cargs.push_back(program.c_str());
-    for (Strings::const_iterator i = args.begin(); i != args.end(); ++i)
-        cargs.push_back(i->c_str());
-    cargs.push_back(0);
-
     /* Create a pipe. */
     Pipe pipe;
     pipe.create();
@@ -912,6 +915,10 @@ string runProgram(Path program, bool searchPath, const Strings & args)
         if (dup2(pipe.writeSide, STDOUT_FILENO) == -1)
             throw SysError("dupping stdout");
 
+        Strings args_(args);
+        args_.push_front(program);
+        auto cargs = stringsToCharPtrs(args_);
+
         if (searchPath)
             execvp(program.c_str(), (char * *) &cargs[0]);
         else
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 8a48a2b7f2..6a84ed8851 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -281,6 +281,11 @@ string runProgram(Path program, bool searchPath = false,
 
 MakeError(ExecError, Error)
 
+/* Convert a list of strings to a null-terminated vector of char
+   *'s. The result must not be accessed beyond the lifetime of the
+   list of strings. */
+std::vector<const char *> stringsToCharPtrs(const Strings & ss);
+
 /* Close all file descriptors except stdin, stdout, stderr, and those
    listed in the given set.  Good practice in child processes. */
 void closeMostFDs(const set<int> & exceptions);