Changed `os::spawn()` to return `Option<int>` instead of `int`. Similar to `os::system()`, `os::spawn()` returned `-1` on error, which is a valid exit code on Windows. Using the same solution as `os::system()`, now when `os::spawn()` fails, `None()` will be returned. Otherwise, the process exit code will be returned.
Review: https://reviews.apache.org/r/65842/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1971a547 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1971a547 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1971a547 Branch: refs/heads/master Commit: 1971a5477f9a0ad9c5c542a0377b32fd7936a219 Parents: 765c834 Author: Akash Gupta <akash-gu...@hotmail.com> Authored: Tue Mar 6 13:11:25 2018 -0800 Committer: Andrew Schwartzmeyer <and...@schwartzmeyer.com> Committed: Tue Mar 6 13:52:47 2018 -0800 ---------------------------------------------------------------------- .../stout/include/stout/os/posix/copyfile.hpp | 11 +++++----- 3rdparty/stout/include/stout/os/posix/shell.hpp | 17 ++++++++-------- .../stout/include/stout/os/windows/shell.hpp | 21 ++++++++------------ 3 files changed, 23 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/1971a547/3rdparty/stout/include/stout/os/posix/copyfile.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/posix/copyfile.hpp b/3rdparty/stout/include/stout/os/posix/copyfile.hpp index 0d9ed5b..bb5ec71 100644 --- a/3rdparty/stout/include/stout/os/posix/copyfile.hpp +++ b/3rdparty/stout/include/stout/os/posix/copyfile.hpp @@ -17,11 +17,12 @@ #include <stout/error.hpp> #include <stout/nothing.hpp> -#include <stout/os/stat.hpp> +#include <stout/option.hpp> #include <stout/path.hpp> #include <stout/stringify.hpp> #include <stout/try.hpp> +#include <stout/os/stat.hpp> namespace os { @@ -56,14 +57,14 @@ inline Try<Nothing> copyfile( return Error("`destination` was a relative path"); } - const int status = os::spawn("cp", {"cp", source, destination}); + const Option<int> status = os::spawn("cp", {"cp", source, destination}); - if (status == -1) { + if (status.isNone()) { return ErrnoError("os::spawn failed"); } - if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) { - return Error("cp failed with status: " + stringify(status)); + if (!(WIFEXITED(status.get()) && WEXITSTATUS(status.get()) == 0)) { + return Error("cp failed with status: " + stringify(status.get())); } return Nothing(); http://git-wip-us.apache.org/repos/asf/mesos/blob/1971a547/3rdparty/stout/include/stout/os/posix/shell.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/posix/shell.hpp b/3rdparty/stout/include/stout/os/posix/shell.hpp index b6e3deb..de6ef23 100644 --- a/3rdparty/stout/include/stout/os/posix/shell.hpp +++ b/3rdparty/stout/include/stout/os/posix/shell.hpp @@ -153,19 +153,20 @@ inline Option<int> system(const std::string& command) } } -// Executes a command by calling "<command> <arguments...>", and -// returns after the command has been completed. Returns 0 if -// succeeds, and -1 on error (e.g., fork/exec/waitpid failed). This -// function is async signal safe. We return int instead of returning a -// Try because Try involves 'new', which is not async signal safe. -inline int spawn( +// Executes a command by calling "<command> <arguments...>", and returns after +// the command has been completed. Returns the exit code on success and `None` +// on error (e.g., fork/exec/waitpid failed). This function is async signal +// safe. We return an `Option<int>` instead of a `Try<int>`, because although +// `Try` does not dynamically allocate, `Error` uses `std::string`, which is +// not async signal safe. +inline Option<int> spawn( const std::string& command, const std::vector<std::string>& arguments) { pid_t pid = ::fork(); if (pid == -1) { - return -1; + return None(); } else if (pid == 0) { // In child process. ::execvp(command.c_str(), os::raw::Argv(arguments)); @@ -175,7 +176,7 @@ inline int spawn( int status; while (::waitpid(pid, &status, 0) == -1) { if (errno != EINTR) { - return -1; + return None(); } } http://git-wip-us.apache.org/repos/asf/mesos/blob/1971a547/3rdparty/stout/include/stout/os/windows/shell.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/shell.hpp b/3rdparty/stout/include/stout/os/windows/shell.hpp index c0b9efa..aacd746 100644 --- a/3rdparty/stout/include/stout/os/windows/shell.hpp +++ b/3rdparty/stout/include/stout/os/windows/shell.hpp @@ -362,9 +362,9 @@ inline int execlp(const char* file, T... t) = delete; // Executes a command by calling "<command> <arguments...>", and -// returns after the command has been completed. Returns process exit code if -// succeeds, and -1 on error. -inline int spawn( +// returns after the command has been completed. Returns the process exit +// code on success and `None` on error. +inline Option<int> spawn( const std::string& command, const std::vector<std::string>& arguments, const Option<std::map<std::string, std::string>>& environment = None()) @@ -374,7 +374,7 @@ inline int spawn( if (process_data.isError()) { LOG(WARNING) << process_data.error(); - return -1; + return None(); } // Wait for the process synchronously. @@ -386,7 +386,7 @@ inline int spawn( process_data->process_handle.get_handle(), &status)) { LOG(WARNING) << "Failed to `GetExitCodeProcess`: " << command; - return -1; + return None(); } // Return the child exit code. @@ -405,12 +405,7 @@ inline int spawn( // preferred if a shell is not required. inline Option<int> system(const std::string& command) { - // TODO(akagup): Change `os::spawn` to return `Option<int>` as well. - int pid = os::spawn(Shell::name, {Shell::arg0, Shell::arg1, command}); - if (pid == -1) { - return None(); - } - return pid; + return os::spawn(Shell::name, {Shell::arg0, Shell::arg1, command}); } @@ -421,7 +416,7 @@ inline int execvp( const std::string& command, const std::vector<std::string>& argv) { - exit(os::spawn(command, argv)); + exit(os::spawn(command, argv).getOrElse(-1)); return -1; } @@ -434,7 +429,7 @@ inline int execvpe( const std::vector<std::string>& argv, const std::map<std::string, std::string>& envp) { - exit(os::spawn(command, argv, envp)); + exit(os::spawn(command, argv, envp).getOrElse(-1)); return -1; }