You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/07/04 17:01:26 UTC
[05/15] mesos git commit: Removed the argv parameter in command
executor helper.
Removed the argv parameter in command executor helper.
It's confusing to pass in 'argv' because it's already contained in
'command'. This patch removed this parameter.
Review: https://reviews.apache.org/r/49415
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/72a47525
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/72a47525
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/72a47525
Branch: refs/heads/master
Commit: 72a475251403e530c3279f435c17cbf8baa9cc31
Parents: 97cc5b5
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 29 17:40:19 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700
----------------------------------------------------------------------
src/launcher/executor.cpp | 11 -----------
src/launcher/posix/executor.cpp | 10 +++++++++-
src/launcher/posix/executor.hpp | 1 -
src/launcher/windows/executor.cpp | 12 +++++++++++-
src/launcher/windows/executor.hpp | 1 -
5 files changed, 20 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 6301025..85de2a1 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -390,18 +390,10 @@ protected:
cout << "Starting task " << task->task_id() << endl;
- // Prepare the argv before fork as it's not async signal safe.
- char **argv = new char*[command.arguments().size() + 1];
- for (int i = 0; i < command.arguments().size(); i++) {
- argv[i] = (char*) command.arguments(i).c_str();
- }
- argv[command.arguments().size()] = nullptr;
-
#ifndef __WINDOWS__
pid = launchTaskPosix(
command,
user,
- argv,
rootfs,
sandboxDirectory,
workingDirectory);
@@ -411,7 +403,6 @@ protected:
// open the reap function will work.
PROCESS_INFORMATION processInformation = launchTaskWindows(
command,
- argv,
rootfs);
pid = processInformation.dwProcessId;
@@ -420,8 +411,6 @@ protected:
processHandle = processInformation.hProcess;
#endif
- delete[] argv;
-
cout << "Forked command at " << pid << endl;
if (task->has_health_check()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index ab1dd93..805280e 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -42,7 +42,6 @@ namespace internal {
pid_t launchTaskPosix(
const mesos::v1::CommandInfo& command,
const Option<string>& user,
- char** argv,
Option<string>& rootfs,
Option<string>& sandboxDirectory,
Option<string>& workingDirectory)
@@ -81,6 +80,13 @@ pid_t launchTaskPosix(
strings::join(", ", command.arguments())).get();
}
+ // Prepare the argv before fork as it's not async signal safe.
+ char **argv = new char*[command.arguments().size() + 1];
+ for (int i = 0; i < command.arguments().size(); i++) {
+ argv[i] = (char*) command.arguments(i).c_str();
+ }
+ argv[command.arguments().size()] = nullptr;
+
pid_t pid;
if ((pid = fork()) == -1) {
ABORT("Failed to fork to run '" + commandString + "'"
@@ -208,6 +214,8 @@ pid_t launchTaskPosix(
ABORT("Failed to exec: " + os::strerror(errno));
}
+ delete[] argv;
+
return pid;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/posix/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.hpp b/src/launcher/posix/executor.hpp
index e5f3c0a..1ff2437 100644
--- a/src/launcher/posix/executor.hpp
+++ b/src/launcher/posix/executor.hpp
@@ -30,7 +30,6 @@ namespace internal {
pid_t launchTaskPosix(
const mesos::v1::CommandInfo& command,
const Option<std::string>& user,
- char** argv,
Option<std::string>& rootfs,
Option<std::string>& sandboxDirectory,
Option<std::string>& workingDirectory);
http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/windows/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/windows/executor.cpp b/src/launcher/windows/executor.cpp
index f6da398..b008052 100644
--- a/src/launcher/windows/executor.cpp
+++ b/src/launcher/windows/executor.cpp
@@ -35,7 +35,6 @@ namespace internal {
PROCESS_INFORMATION launchTaskWindows(
const CommandInfo& command,
- char** argv,
Option<string>& rootfs)
{
PROCESS_INFORMATION processInfo;
@@ -67,7 +66,18 @@ PROCESS_INFORMATION launchTaskWindows(
} else {
// Not a shell command, execute as-is.
executable = command.value();
+
+ // TODO(jieyu): Consider allowing os::stringify_args to take
+ // `command.arguments()` directly.
+ char **argv = new char*[command.arguments().size() + 1];
+ for (int i = 0; i < command.arguments().size(); i++) {
+ argv[i] = (char*) command.arguments(i).c_str();
+ }
+ argv[command.arguments().size()] = nullptr;
+
commandLine = os::stringify_args(argv);
+
+ delete[] argv;
}
cout << commandLine << endl;
http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/windows/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/windows/executor.hpp b/src/launcher/windows/executor.hpp
index 04484b6..bae44f5 100644
--- a/src/launcher/windows/executor.hpp
+++ b/src/launcher/windows/executor.hpp
@@ -29,7 +29,6 @@ namespace internal {
PROCESS_INFORMATION launchTaskWindows(
const CommandInfo& command,
- char** argv,
Option<std::string>& rootfs);
} // namespace internal {