Refactored Docker::Container::pid() to return an Option. Safer to check for 'None' than 0. Also fixed a compilation bug comparing signed vs unsigned values with CHECK_NE.
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/38e4752a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/38e4752a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/38e4752a Branch: refs/heads/master Commit: 38e4752af9a8900883f1650f98e15d9903aa17c8 Parents: 5d78d0c Author: Benjamin Hindman <[email protected]> Authored: Sun Jun 29 17:51:38 2014 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Mon Aug 4 15:08:16 2014 -0700 ---------------------------------------------------------------------- src/docker/docker.cpp | 14 +++++++++----- src/docker/docker.hpp | 6 +++--- src/slave/containerizer/docker.cpp | 6 +++--- 3 files changed, 15 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/38e4752a/src/docker/docker.cpp ---------------------------------------------------------------------- diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index 070279e..0652a53 100644 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -55,7 +55,7 @@ string Docker::Container::name() const return value.as<JSON::String>().value; } -pid_t Docker::Container::pid() const +Option<pid_t> Docker::Container::pid() const { map<string, JSON::Value>::const_iterator state = json.values.find("State"); @@ -66,9 +66,13 @@ pid_t Docker::Container::pid() const map<string, JSON::Value>::const_iterator entry = value.as<JSON::Object>().values.find("Pid"); CHECK(entry != json.values.end()); - JSON::Value pid = entry->second; - CHECK(pid.is<JSON::Number>()); - return pid_t(pid.as<JSON::Number>().value); + value = entry->second; + CHECK(value.is<JSON::Number>()); + pid_t pid = pid_t(value.as<JSON::Number>().value); + if (pid == 0) { + return None(); + } + return pid; } Future<Option<int> > Docker::run( @@ -288,7 +292,7 @@ Future<list<Docker::Container> > Docker::_ps( vector<string> lines = strings::tokenize(output.get(), "\n"); // Skip the header. - CHECK_NE(0, lines.size()); + CHECK(!lines.empty()); lines.erase(lines.begin()); list<Future<Docker::Container> > futures; http://git-wip-us.apache.org/repos/asf/mesos/blob/38e4752a/src/docker/docker.hpp ---------------------------------------------------------------------- diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp index 56b6c61..6643036 100644 --- a/src/docker/docker.hpp +++ b/src/docker/docker.hpp @@ -47,9 +47,9 @@ public: // Returns the name of the container. std::string name() const; - // Returns the Pid of the container. - // Note: If it returns 0, it means the container is not running. - pid_t pid() const; + // Returns the Pid of the container, or None if the container is + // not running. + Option<pid_t> pid() const; private: JSON::Object json; // JSON returned from 'docker inspect'. http://git-wip-us.apache.org/repos/asf/mesos/blob/38e4752a/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 87510fa..24255e6 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -650,12 +650,12 @@ Future<ResourceStatistics> DockerContainerizerProcess::_usage( const ContainerID& containerId, const Future<Docker::Container> container) { - pid_t pid = container.get().pid(); - if (pid == 0) { + Option<pid_t> pid = container.get().pid(); + if (pid.isNone()) { return Failure("Container is not running"); } Try<ResourceStatistics> usage = - mesos::internal::usage(pid, true, true); + mesos::internal::usage(pid.get(), true, true); if (usage.isError()) { return Failure(usage.error()); }
