[mesos] 03/03: Avoided unnecessary [] map operators in the master.

[bmahler]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c762478ebcbfbcadc513fd6ec57fcdd7a84bbf69
Author: Benjamin Mahler <bmah...@apache.org>
AuthorDate: Tue Sep 1 15:08:08 2020 -0400

    Avoided unnecessary [] map operators in the master.
    
    To avoid the potential for accidental insertion into the maps,
    we prefer to use the .at operator for const access.
    
    Review: https://reviews.apache.org/r/72832
---
 src/master/master.cpp | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 97654b5..438ef98 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -10445,7 +10445,7 @@ void Master::removeFramework(Framework* framework)
 
     if (slave != nullptr) {
       foreachkey (const ExecutorID& executorId,
-                  utils::copy(framework->executors[slaveId])) {
+                  utils::copy(framework->executors.at(slaveId))) {
         removeExecutor(slave, framework->id(), executorId);
       }
     }
@@ -10584,7 +10584,7 @@ void Master::removeFramework(Slave* slave, Framework* 
framework)
   // for proper resource accounting.
   if (slave->executors.contains(framework->id())) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[framework->id()])) {
+                utils::copy(slave->executors.at(framework->id()))) {
       removeExecutor(slave, framework->id(), executorId);
     }
   }
@@ -10805,8 +10805,11 @@ void Master::_removeSlave(
   // Transition the tasks to lost and remove them.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->tasks)) {
     Framework* framework = getFramework(frameworkId);
+    CHECK(framework != nullptr)
+      << "Framework " << frameworkId << " not found while removing agent "
+      << *slave << "; agent tasks: " << slave->tasks;
 
-    foreachvalue (Task* task, utils::copy(slave->tasks[frameworkId])) {
+    foreachvalue (Task* task, utils::copy(slave->tasks.at(frameworkId))) {
       // TODO(bmahler): Differentiate between agent removal reasons
       // (e.g. unhealthy vs. unregistered for maintenance).
       const StatusUpdate& update = protobuf::createStatusUpdate(
@@ -10824,7 +10827,7 @@ void Master::_removeSlave(
       updateTask(task, update);
       removeTask(task);
 
-      if (framework == nullptr || !framework->connected()) {
+      if (!framework->connected()) {
         LOG(WARNING) << "Dropping update " << update
                      << " for unknown framework " << frameworkId;
       } else {
@@ -10836,7 +10839,7 @@ void Master::_removeSlave(
   // Remove executors from the slave for proper resource accounting.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->executors)) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[frameworkId])) {
+                utils::copy(slave->executors.at(frameworkId))) {
       removeExecutor(slave, frameworkId, executorId);
     }
   }
@@ -10951,7 +10954,7 @@ void Master::__removeSlave(
       newTaskReason = TaskStatus::REASON_SLAVE_REMOVED_BY_OPERATOR;
     }
 
-    foreachvalue (Task* task, utils::copy(slave->tasks[frameworkId])) {
+    foreachvalue (Task* task, utils::copy(slave->tasks.at(frameworkId))) {
       const StatusUpdate& update = protobuf::createStatusUpdate(
           task->framework_id(),
           task->slave_id(),
@@ -10985,7 +10988,7 @@ void Master::__removeSlave(
   // Remove executors from the slave for proper resource accounting.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->executors)) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[frameworkId])) {
+                utils::copy(slave->executors.at(frameworkId))) {
       removeExecutor(slave, frameworkId, executorId);
     }
   }
@@ -12618,8 +12621,8 @@ void Slave::removeTask(Task* task)
     }
   }
 
-  tasks[frameworkId].erase(taskId);
-  if (tasks[frameworkId].empty()) {
+  tasks.at(frameworkId).erase(taskId);
+  if (tasks.at(frameworkId).empty()) {
     tasks.erase(frameworkId);
   }