From 436a2af9265f32c1310ef418c0b7a3ddad08066e Mon Sep 17 00:00:00 2001
From: Martin Quinson <martin.quinson@loria.fr>
Date: Thu, 10 May 2018 11:45:49 +0200
Subject: [PATCH] privatize ExecImpl::timeoutDetector and partially
 ExecImpl::surfAction

I fail to understand why a direct access to ExecImpl::surfAction is
mandatory
---
 src/kernel/activity/ExecImpl.cpp | 15 +++++++++++++--
 src/kernel/activity/ExecImpl.hpp |  9 +++++++--
 src/simix/libsmx.cpp             | 12 ++++--------
 src/simix/smx_host.cpp           |  2 +-
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/kernel/activity/ExecImpl.cpp b/src/kernel/activity/ExecImpl.cpp
index 259d5cbde8..ac22116526 100644
--- a/src/kernel/activity/ExecImpl.cpp
+++ b/src/kernel/activity/ExecImpl.cpp
@@ -45,16 +45,22 @@ simgrid::kernel::activity::ExecImpl::~ExecImpl()
 void simgrid::kernel::activity::ExecImpl::suspend()
 {
   XBT_VERB("This exec is suspended (remain: %f)", surfAction_->get_remains());
-  if (surfAction_)
+  if (surfAction_ != nullptr)
     surfAction_->suspend();
 }
 
 void simgrid::kernel::activity::ExecImpl::resume()
 {
   XBT_VERB("This exec is resumed (remain: %f)", surfAction_->get_remains());
-  if (surfAction_)
+  if (surfAction_ != nullptr)
     surfAction_->resume();
 }
+void simgrid::kernel::activity::ExecImpl::cancel()
+{
+  XBT_VERB("This exec %p is canceled", this);
+  if (surfAction_ != nullptr)
+    surfAction_->cancel();
+}
 
 double simgrid::kernel::activity::ExecImpl::get_remaining()
 {
@@ -78,6 +84,11 @@ void simgrid::kernel::activity::ExecImpl::set_bound(double bound)
   if (surfAction_)
     surfAction_->set_bound(bound);
 }
+void simgrid::kernel::activity::ExecImpl::set_priority(double priority)
+{
+  if (surfAction_)
+    surfAction_->set_priority(priority);
+}
 
 void simgrid::kernel::activity::ExecImpl::post()
 {
diff --git a/src/kernel/activity/ExecImpl.hpp b/src/kernel/activity/ExecImpl.hpp
index 6aa17c9ef3..59876f3ae4 100644
--- a/src/kernel/activity/ExecImpl.hpp
+++ b/src/kernel/activity/ExecImpl.hpp
@@ -21,16 +21,21 @@ public:
                     s4u::Host* host);
   void suspend() override;
   void resume() override;
+  void cancel();
   void post() override;
   double get_remaining();
   double get_remaining_ratio();
   void set_bound(double bound);
+  void set_priority(double priority);
   virtual ActivityImpl* migrate(s4u::Host* to);
 
   /* The host where the execution takes place. nullptr means this is a parallel exec (and only surf knows the hosts) */
-  s4u::Host* host_                  = nullptr;
-  resource::Action* surfAction_     = nullptr; /* The Surf execution action encapsulated */
+  s4u::Host* host_ = nullptr;
+  resource::Action* surfAction_; /* The Surf execution action encapsulated */
+private:
   resource::Action* timeoutDetector = nullptr;
+
+public:
   static simgrid::xbt::signal<void(kernel::activity::ExecImplPtr)> onCreation;
   static simgrid::xbt::signal<void(kernel::activity::ExecImplPtr)> onCompletion;
   static simgrid::xbt::signal<void(simgrid::kernel::activity::ExecImplPtr, simgrid::s4u::Host*)> onMigration;
diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp
index 3647e351e8..5b479f98e9 100644
--- a/src/simix/libsmx.cpp
+++ b/src/simix/libsmx.cpp
@@ -117,12 +117,10 @@ void simcall_execution_cancel(smx_activity_t execution)
 {
   simgrid::kernel::activity::ExecImplPtr exec =
       boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-  if (not exec->surfAction_)
+  if (exec->surfAction_ == nullptr) // FIXME: One test fails if I remove this, but I don't get why...
     return;
   simgrid::simix::kernelImmediate([exec] {
-    XBT_DEBUG("Cancel synchro %p", exec.get());
-    if (exec->surfAction_)
-      exec->surfAction_->cancel();
+    exec->cancel();
   });
 }
 
@@ -142,8 +140,7 @@ void simcall_execution_set_priority(smx_activity_t execution, double priority)
 
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-    if (exec->surfAction_)
-      exec->surfAction_->set_priority(priority);
+    exec->set_priority(priority);
   });
 }
 
@@ -160,8 +157,7 @@ void simcall_execution_set_bound(smx_activity_t execution, double bound)
   simgrid::simix::kernelImmediate([execution, bound] {
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-    if (exec->surfAction_)
-      exec->surfAction_->set_bound(bound);
+    exec->set_bound(bound);
   });
 }
 
diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp
index 82a0566eea..cdd6351332 100644
--- a/src/simix/smx_host.cpp
+++ b/src/simix/smx_host.cpp
@@ -139,7 +139,7 @@ boost::intrusive_ptr<simgrid::kernel::activity::ExecImpl>
 SIMIX_execution_start(const char* name, double flops_amount, double priority, double bound, sg_host_t host)
 {
   /* set surf's action */
-  simgrid::kernel::resource::Action* surf_action      = nullptr;
+  simgrid::kernel::resource::Action* surf_action = nullptr;
   if (not MC_is_active() && not MC_record_replay_is_active()) {
     surf_action = host->pimpl_cpu->execution_start(flops_amount);
     surf_action->set_priority(priority);
-- 
2.20.1