You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/04/25 23:35:48 UTC

[07/48] mesos git commit: Disallowed non-`const` iteration over `Resources`.

Disallowed non-`const` iteration over `Resources`.

Review: https://reviews.apache.org/r/42876


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/96acace1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/96acace1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/96acace1

Branch: refs/heads/0.27.x
Commit: 96acace1df51ca3ea23a1dea45b9988e76c85018
Parents: bdd8815
Author: Michael Park <mp...@apache.org>
Authored: Wed Jan 27 14:04:09 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Wed Jan 27 20:04:51 2016 -0800

----------------------------------------------------------------------
 include/mesos/resources.hpp                 | 18 +++++++++++++++---
 include/mesos/v1/resources.hpp              | 18 +++++++++++++++---
 src/master/allocator/mesos/hierarchical.cpp |  7 ++++---
 src/master/http.cpp                         |  5 +++--
 src/slave/resource_estimators/fixed.cpp     |  4 ++--
 src/tests/resources_tests.cpp               |  4 ++--
 6 files changed, 41 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index cc8fef9..6bfac2e 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -317,14 +317,26 @@ public:
   // which holds the ephemeral ports allocation logic.
   Option<Value::Ranges> ephemeral_ports() const;
 
-  typedef google::protobuf::RepeatedPtrField<Resource>::iterator
+  // NOTE: Non-`const` `iterator`, `begin()` and `end()` are __intentionally__
+  // defined with `const` semantics in order to prevent mutable access to the
+  // `Resource` objects within `resources`.
+  typedef google::protobuf::RepeatedPtrField<Resource>::const_iterator
   iterator;
 
   typedef google::protobuf::RepeatedPtrField<Resource>::const_iterator
   const_iterator;
 
-  iterator begin() { return resources.begin(); }
-  iterator end() { return resources.end(); }
+  const_iterator begin()
+  {
+    using google::protobuf::RepeatedPtrField;
+    return static_cast<const RepeatedPtrField<Resource>&>(resources).begin();
+  }
+
+  const_iterator end()
+  {
+    using google::protobuf::RepeatedPtrField;
+    return static_cast<const RepeatedPtrField<Resource>&>(resources).end();
+  }
 
   const_iterator begin() const { return resources.begin(); }
   const_iterator end() const { return resources.end(); }

http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/include/mesos/v1/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index f489297..5a88c07 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -317,14 +317,26 @@ public:
   // which holds the ephemeral ports allocation logic.
   Option<Value::Ranges> ephemeral_ports() const;
 
-  typedef google::protobuf::RepeatedPtrField<Resource>::iterator
+  // NOTE: Non-`const` `iterator`, `begin()` and `end()` are __intentionally__
+  // defined with `const` semantics in order to prevent mutable access to the
+  // `Resource` objects within `resources`.
+  typedef google::protobuf::RepeatedPtrField<Resource>::const_iterator
   iterator;
 
   typedef google::protobuf::RepeatedPtrField<Resource>::const_iterator
   const_iterator;
 
-  iterator begin() { return resources.begin(); }
-  iterator end() { return resources.end(); }
+  const_iterator begin()
+  {
+    using google::protobuf::RepeatedPtrField;
+    return static_cast<const RepeatedPtrField<Resource>&>(resources).begin();
+  }
+
+  const_iterator end()
+  {
+    using google::protobuf::RepeatedPtrField;
+    return static_cast<const RepeatedPtrField<Resource>&>(resources).end();
+  }
 
   const_iterator begin() const { return resources.begin(); }
   const_iterator end() const { return resources.end(); }

http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index dbc77a2..1a07d69 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1160,13 +1160,14 @@ void HierarchicalAllocatorProcess::allocate(
   auto getQuotaRoleAllocatedResources = [this](const std::string& role) {
     CHECK(quotas.contains(role));
 
-    Resources resources = quotaRoleSorter->allocationScalars(role);
-
     // Strip the reservation and persistent volume info.
-    foreach (Resource& resource, resources) {
+    Resources resources;
+
+    foreach (Resource resource, quotaRoleSorter->allocationScalars(role)) {
       resource.set_role("*");
       resource.clear_reservation();
       resource.clear_disk();
+      resources += resource;
     }
 
     return resources;

http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 12c1fe5..54264f6 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -708,10 +708,11 @@ string Master::Http::CREATE_VOLUMES_HELP()
 
 static Resources removeDiskInfos(const Resources& resources)
 {
-  Resources result = resources;
+  Resources result;
 
-  foreach (Resource& resource, result) {
+  foreach (Resource resource, resources) {
     resource.clear_disk();
+    result += resource;
   }
 
   return result;

http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/src/slave/resource_estimators/fixed.cpp
----------------------------------------------------------------------
diff --git a/src/slave/resource_estimators/fixed.cpp b/src/slave/resource_estimators/fixed.cpp
index aedd08d..c858a48 100644
--- a/src/slave/resource_estimators/fixed.cpp
+++ b/src/slave/resource_estimators/fixed.cpp
@@ -68,11 +68,11 @@ class FixedResourceEstimator : public ResourceEstimator
 {
 public:
   FixedResourceEstimator(const Resources& _totalRevocable)
-    : totalRevocable(_totalRevocable)
   {
     // Mark all resources as revocable.
-    foreach (Resource& resource, totalRevocable) {
+    foreach (Resource resource, _totalRevocable) {
       resource.mutable_revocable();
+      totalRevocable += resource;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/96acace1/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 54a4fa8..4b25e82 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -351,7 +351,7 @@ TEST(ResourcesTest, ParsingFromJSONWithRoles)
   EXPECT_TRUE(resources.contains(Resources(*cpus)));
   EXPECT_EQ(145.54, resources.cpus().get());
 
-  foreach (Resource& resource, resources) {
+  foreach (const Resource& resource, resources) {
     if (resource.role() == "role1") {
       EXPECT_EQ(91.1, resource.scalar().value());
     } else {
@@ -396,7 +396,7 @@ TEST(ResourcesTest, ParsingFromJSONWithRoles)
 
   resources = resourcesTry.get();
 
-  foreach (Resource& resource, resources) {
+  foreach (const Resource& resource, resources) {
     if (resource.role() == "role1") {
       EXPECT_EQ(Value::RANGES, resource.type());
       EXPECT_EQ(2, resource.ranges().range_size());