fewer temporary allocations in subprocess

ostd/process.hh

@@ -383,7 +383,7 @@ private:
     template<typename InputRange>
     void open_full(string_range cmd, InputRange args, bool use_path) {
         static_assert(
-            std::is_constructible_v<std::string, range_reference_t<InputRange>>,
+            std::is_constructible_v<string_range, range_reference_t<InputRange>>,
             "The arguments must be strings"
         );
 
@@ -391,25 +391,20 @@ private:
             throw subprocess_error{"another child process already running"};
         }
 
-        std::vector<std::string> argv;
+        open_impl(use_path, cmd, [](string_range &arg, void *data) {
-        for (; !args.empty(); args.pop_front()) {
+            InputRange &argr = *static_cast<InputRange *>(data);
-            argv.emplace_back(args.front());
+            if (argr.empty()) {
-        }
+                return false;
-        if (argv.empty()) {
-            throw subprocess_error{"no arguments given"};
-        }
-        if (cmd.empty()) {
-            cmd = argv[0];
-            if (cmd.empty()) {
-                throw subprocess_error{"no command given"};
             }
-        }
+            arg = string_range{argr.front()};
-        open_impl(std::string{cmd}, argv, use_path);
+            argr.pop_front();
+            return true;
+        }, &args);
     }
 
     void open_impl(
-        std::string const &cmd, std::vector<std::string> const &args,
+        bool use_path, string_range cmd,
-        bool use_path
+        bool (*func)(string_range &, void *), void *data
     );
 
     void reset();

src/posix/process.cc

@@ -132,18 +132,55 @@ struct pipe {
 };
 
 OSTD_EXPORT void subprocess::open_impl(
-    std::string const &cmd, std::vector<std::string> const &args,
+    bool use_path, string_range cmd,
-    bool use_path
+    bool (*func)(string_range &, void *), void *datap
 ) {
     if (use_in == subprocess_stream::STDOUT) {
         throw subprocess_error{"could not redirect stdin to stdout"};
     }
 
-    auto argp = std::make_unique<char *[]>(args.size() + 1);
+    struct argpv {
-    for (std::size_t i = 0; i < args.size(); ++i) {
+        char *cmd = nullptr;
-        argp[i] = const_cast<char *>(args[i].data());
+        std::vector<char *> vec;
+        ~argpv() {
+            auto vs = vec.size();
+            if (!vs) {
+                return;
+            }
+            if (cmd && (cmd != vec[0])) {
+                delete[] cmd;
+            }
+            for (std::size_t i = 0; i < (vs - 1); ++i) {
+                delete[] vec[i];
+            }
+        }
+    } argp;
+
+    for (string_range r; func(r, datap);) {
+        auto sz = r.size();
+        char *str = new char[sz + 1];
+        string_range::traits_type::copy(str, r.data(), sz)[sz] = '\0';
+        argp.vec.push_back(str);
     }
-    argp[args.size()] = nullptr;
+
+    if (argp.vec.empty()) {
+        throw subprocess_error{"no arguments given"};
+    }
+
+    if (cmd.empty()) {
+        argp.cmd = argp.vec[0];
+        if (!*argp.cmd) {
+            throw subprocess_error{"no command given"};
+        }
+    } else {
+        auto sz = cmd.size();
+        char *str = new char[sz + 1];
+        string_range::traits_type::copy(str, cmd.data(), sz)[sz] = '\0';
+        argp.cmd = str;
+    }
+
+    /* terminate */
+    argp.vec.push_back(nullptr);
 
     /* fd_errno used to detect if exec failed */
     pipe fd_errno, fd_stdin, fd_stdout, fd_stderr;
@@ -189,9 +226,9 @@ OSTD_EXPORT void subprocess::open_impl(
             }
         }
         if (use_path) {
-            execvp(cmd.data(), argp.get());
+            execvp(argp.cmd, argp.vec.data());
         } else {
-            execv(cmd.data(), argp.get());
+            execv(argp.cmd, argp.vec.data());
         }
         /* exec has returned, so error has occured */
         fd_errno.write_errno();

src/win32/process.cc

@@ -21,6 +21,7 @@
 #include <fcntl.h>
 #include <windows.h>
 
+#include "ostd/algprithm.hh"
 #include "ostd/process.hh"
 #include "ostd/format.hh"
 
@@ -237,47 +238,93 @@ static std::wstring resolve_file(wchar_t const *cmd) {
  *
  * we need to replicate this awful behavior here, hence the extra code
  */
-static std::string concat_args(std::vector<std::string> const &args) {
+static std::unique_ptr<wchar_t[]> concat_args(
+    string_range cmd, bool (*func)(string_range &, void *), void *datap,
+    std::wstring &cmdpath
+) {
     std::string ret;
-    for (auto &s: args) {
+
+    string_range p;
+    if (!func(p, datap)) {
+        throw subprocess_error{"no arguments given"};
+    }
+    if (!cmd.size()) {
+        cmd = p;
+        if (!cmd.size()) {
+            throw subprocess_error{"no command given"};
+        }
+    }
+
+    /* convert and optionally resolve PATH and other lookup locations */
+    {
+        std::unique_ptr<wchar_t[]> wcmd{new wchar_t[cmd.size() + 1]};
+        auto req = MultiByteToWideChar(
+            CP_UTF8, 0, cmd.data(), cmd.size(), wcmd.get(), cmd.size() + 1
+        );
+        if (!req) {
+            throw subprocess_error{"unicode conversion failed"};
+        }
+        wcmd.get()[req] = '\0';
+        if (!use_path) {
+            cmdpath = wcmd.get();
+        } else {
+            cmdpath = resolve_file(wcmd.get());
+        }
+    }
+
+    /* concat the args */
+    for (bool has = true; has; has = func(p, datap)) {
         if (!ret.empty()) {
             ret += ' ';
         }
         ret += '\"';
-        for (char const *sp = s.data();;) {
+        for (;;) {
-            char const *p = strpbrk(sp, "\"\\");
+            auto found = ostd::find_one_of(p, string_range{"\"\\"});
-            if (!p) {
+            if (found.empty()) {
-                ret += sp;
+                ret.append(p.iter_begin(), p.iter_end());
                 break;
             }
-            ret.append(sp, p);
+            auto sl = p.slice(0, p.size() - found.size());
-            if (*p == '\"') {
+            ret.append(sl.iter_begin(), sl.iter_end());
+            if (found.front() == '\"') {
                 /* not preceded by \, so it's safe */
                 ret += "\\\"";
-                ++p;
+                found.pop_front();
             } else {
                 /* handle any sequence of \ optionally followed by a " */
-                char const *op = p;
+                std::size_t nsl = 0;
-                while (*p == '\\') {
+                while (found.front() == '\\') {
-                    ++p;
+                    ++nsl;
+                    found.pop_front();
                 }
-                if (*p == '\"') {
+                if (!found.empty() && (found.front() == '\"')) {
                     /* double all the backslashes plus one for the " */
-                    ret.append((p - op) * 2 + 1, '\\');
+                    ret.append(nsl * 2 + 1, '\\');
                     ret += '\"';
                 } else {
-                    ret.append(p - op, '\\');
+                    /* double if the backslashes were at the end of the arg */
+                    ret.append(nsl * (found.empty() + 1), '\\');
                 }
             }
-            sp = p;
+            p = found;
         }
         ret += '\"';
     }
-    return ret;
+
+    /* convert to UTF-16 */
+    std::unique_ptr<wchar_t[]> cmdline{new wchar_t[ret.size() + 1]};
+    if (!MultiByteToWideChar(
+        CP_UTF8, 0, ret.data(), ret.size() + 1, cmdline.get(), ret.size() + 1
+    )) {
+        throw subprocess_error{"unicode conversion failed"};
+    }
+
+    return cmdline;
 }
 
 OSTD_EXPORT void subprocess::open_impl(
-    std::string const &cmd, std::vector<std::string> const &args, bool use_path
+    bool use_path, string_range cmd,
+    bool (*func)(string_range &, void *), void *datap
 ) {
     if (use_in == subprocess_stream::STDOUT) {
         throw subprocess_error{"could not redirect stdin to stdout"};
@@ -369,30 +416,7 @@ OSTD_EXPORT void subprocess::open_impl(
     si.dwFlags |= STARTF_USESTDHANDLES;
 
     std::wstring cmdpath;
-    /* convert and optionally resolve PATH and other lookup locations */
+    auto cmdline = concat_args(cmd, func, datap, cmdpath);
-    {
-        std::unique_ptr<wchar_t[]> wcmd{new wchar_t[cmd.size() + 1]};
-        if (!MultiByteToWideChar(
-            CP_UTF8, 0, cmd.data(), cmd.size() + 1, wcmd.get(), cmd.size() + 1
-        )) {
-            throw subprocess_error{"unicode conversion failed"};
-        }
-        if (!use_path) {
-            cmdpath = wcmd.get();
-        } else {
-            cmdpath = resolve_file(wcmd.get());
-        }
-    }
-
-    /* cmdline gets an ordinary conversion... */
-    auto astr = concat_args(args);
-
-    std::unique_ptr<wchar_t[]> cmdline{new wchar_t[astr.size() + 1]};
-    if (!MultiByteToWideChar(
-        CP_UTF8, 0, astr.data(), astr.size() + 1, cmdline.get(), astr.size() + 1
-    )) {
-        throw subprocess_error{"unicode conversion failed"};
-    }
 
     /* owned by CreateProcess, do not close explicitly */
     pipe_in.p_r = nullptr;