[sonar] don't mix public/private data members

diff --git a/src/kernel/activity/ConditionVariableImpl.cpp b/src/kernel/activity/ConditionVariableImpl.cpp
index 976b72e..04d034f 100644 (file)
--- a/src/kernel/activity/ConditionVariableImpl.cpp
+++ b/src/kernel/activity/ConditionVariableImpl.cpp
@@ -28,9 +28,6 @@ namespace simgrid {
 namespace kernel {
 namespace activity {
 
-ConditionVariableImpl::ConditionVariableImpl() : cond_(this) {}
-ConditionVariableImpl::~ConditionVariableImpl() = default;
-
 /**
  * @brief Signalizes a condition.
  *
@@ -85,7 +82,7 @@ void ConditionVariableImpl::wait(smx_mutex_t mutex, double timeout, actor::Actor
 
   /* If there is a mutex unlock it */
   if (mutex != nullptr) {
-    xbt_assert(mutex->owner_ == issuer,
+    xbt_assert(mutex->get_owner() == issuer,
                "Actor %s cannot wait on ConditionVariable %p since it does not own the provided mutex %p",
                issuer->get_cname(), this, mutex);
     mutex_ = mutex;

diff --git a/src/kernel/activity/ConditionVariableImpl.hpp b/src/kernel/activity/ConditionVariableImpl.hpp
index 8ad7766..eb6937b 100644 (file)
--- a/src/kernel/activity/ConditionVariableImpl.hpp
+++ b/src/kernel/activity/ConditionVariableImpl.hpp
@@ -15,22 +15,23 @@ namespace kernel {
 namespace activity {
 
 class XBT_PUBLIC ConditionVariableImpl {
-public:
-  ConditionVariableImpl();
-  ~ConditionVariableImpl();
-
-  actor::SynchroList sleeping_; /* list of sleeping processes */
   MutexImpl* mutex_ = nullptr;
-  s4u::ConditionVariable cond_;
-
-  void broadcast();
-  void signal();
-  void wait(MutexImpl* mutex, double timeout, actor::ActorImpl* issuer);
+  s4u::ConditionVariable piface_;
+  actor::SynchroList sleeping_; /* list of sleeping actors*/
 
-private:
   std::atomic_int_fast32_t refcount_{1};
   friend void intrusive_ptr_add_ref(ConditionVariableImpl* cond);
   friend void intrusive_ptr_release(ConditionVariableImpl* cond);
+
+public:
+  ConditionVariableImpl() : piface_(this){};
+  ~ConditionVariableImpl() = default;
+
+  void remove_sleeping_actor(actor::ActorImpl& actor) { xbt::intrusive_erase(sleeping_, actor); }
+  s4u::ConditionVariable* get_iface() { return &piface_; }
+  void broadcast();
+  void signal();
+  void wait(MutexImpl* mutex, double timeout, actor::ActorImpl* issuer);
 };
 } // namespace activity
 } // namespace kernel

diff --git a/src/kernel/activity/MutexImpl.hpp b/src/kernel/activity/MutexImpl.hpp
index 978fb0e..ba85ad0 100644 (file)
--- a/src/kernel/activity/MutexImpl.hpp
+++ b/src/kernel/activity/MutexImpl.hpp
@@ -18,6 +18,9 @@ class XBT_PUBLIC MutexImpl {
   std::atomic_int_fast32_t refcount_{1};
   s4u::Mutex piface_;
   bool locked_ = false;
+  actor::ActorImpl* owner_ = nullptr;
+  // List of sleeping actors:
+  actor::SynchroList sleeping_;
 
 public:
   MutexImpl() : piface_(this) {}
@@ -32,9 +35,8 @@ public:
   MutexImpl* ref();
   void unref();
 
-  actor::ActorImpl* owner_ = nullptr;
-  // List of sleeping actors:
-  actor::SynchroList sleeping_;
+  void remove_sleeping_actor(actor::ActorImpl& actor) { xbt::intrusive_erase(sleeping_, actor); }
+  actor::ActorImpl* get_owner() const { return owner_; }
 
   // boost::intrusive_ptr<Mutex> support:
   friend void intrusive_ptr_add_ref(MutexImpl* mutex)

diff --git a/src/kernel/activity/SemaphoreImpl.hpp b/src/kernel/activity/SemaphoreImpl.hpp
index b6acb1d..7a61019 100644 (file)
--- a/src/kernel/activity/SemaphoreImpl.hpp
+++ b/src/kernel/activity/SemaphoreImpl.hpp
@@ -19,10 +19,9 @@ namespace activity {
 class XBT_PUBLIC SemaphoreImpl {
   std::atomic_int_fast32_t refcount_{1};
   unsigned int value_;
-
-public:
   actor::SynchroList sleeping_; /* list of sleeping actors*/
 
+public:
   explicit SemaphoreImpl(unsigned int value) : value_(value){};
   ~SemaphoreImpl() = default;
 
@@ -32,8 +31,10 @@ public:
   void acquire(actor::ActorImpl* issuer, double timeout);
   void release();
   bool would_block() { return (value_ == 0); }
+  void remove_sleeping_actor(actor::ActorImpl& actor) { xbt::intrusive_erase(sleeping_, actor); }
 
   unsigned int get_capacity() { return value_; }
+  bool is_used() { return not sleeping_.empty(); }
 
   friend void intrusive_ptr_add_ref(SemaphoreImpl* sem)
   {

diff --git a/src/kernel/activity/SynchroRaw.cpp b/src/kernel/activity/SynchroRaw.cpp
index edff83a..6b1aeca 100644 (file)
--- a/src/kernel/activity/SynchroRaw.cpp
+++ b/src/kernel/activity/SynchroRaw.cpp
@@ -79,24 +79,24 @@ void RawImpl::finish()
 
   switch (simcall->call_) {
     case SIMCALL_MUTEX_LOCK:
-      xbt::intrusive_erase(simcall_mutex_lock__get__mutex(simcall)->sleeping_, *simcall->issuer_);
+      simcall_mutex_lock__get__mutex(simcall)->remove_sleeping_actor(*simcall->issuer_);
       break;
 
     case SIMCALL_COND_WAIT:
-      xbt::intrusive_erase(simcall_cond_wait__get__cond(simcall)->sleeping_, *simcall->issuer_);
+      simcall_cond_wait_timeout__get__cond(simcall)->remove_sleeping_actor(*simcall->issuer_);
       break;
 
     case SIMCALL_COND_WAIT_TIMEOUT:
-      xbt::intrusive_erase(simcall_cond_wait_timeout__get__cond(simcall)->sleeping_, *simcall->issuer_);
+      simcall_cond_wait_timeout__get__cond(simcall)->remove_sleeping_actor(*simcall->issuer_);
       simcall_cond_wait_timeout__set__result(simcall, 1); // signal a timeout
       break;
 
     case SIMCALL_SEM_ACQUIRE:
-      xbt::intrusive_erase(simcall_sem_acquire__get__sem(simcall)->sleeping_, *simcall->issuer_);
+      simcall_sem_acquire_timeout__get__sem(simcall)->remove_sleeping_actor(*simcall->issuer_);
       break;
 
     case SIMCALL_SEM_ACQUIRE_TIMEOUT:
-      xbt::intrusive_erase(simcall_sem_acquire_timeout__get__sem(simcall)->sleeping_, *simcall->issuer_);
+      simcall_sem_acquire_timeout__get__sem(simcall)->remove_sleeping_actor(*simcall->issuer_);
       simcall_sem_acquire_timeout__set__result(simcall, 1); // signal a timeout
       break;
 

diff --git a/src/mc/mc_base.cpp b/src/mc/mc_base.cpp
index 5612154..d3b1935 100644 (file)
--- a/src/mc/mc_base.cpp
+++ b/src/mc/mc_base.cpp
@@ -118,9 +118,9 @@ bool actor_is_enabled(smx_actor_t actor)
     case SIMCALL_MUTEX_LOCK: {
       const kernel::activity::MutexImpl* mutex = simcall_mutex_lock__get__mutex(req);
 
-      if (mutex->owner_ == nullptr)
+      if (mutex->get_owner() == nullptr)
         return true;
-      return mutex->owner_->get_pid() == req->issuer_->get_pid();
+      return mutex->get_owner()->get_pid() == req->issuer_->get_pid();
     }
 
     case SIMCALL_SEM_ACQUIRE: {

diff --git a/src/mc/mc_request.cpp b/src/mc/mc_request.cpp
index 3723ec3..3c663e4 100644 (file)
--- a/src/mc/mc_request.cpp
+++ b/src/mc/mc_request.cpp
@@ -325,9 +325,9 @@ std::string simgrid::mc::request_to_string(smx_simcall_t req, int value, simgrid
                                                         ? simcall_mutex_lock__get__mutex(req)
                                                         : simcall_mutex_trylock__get__mutex(req)));
       args = bprintf("locked = %d, owner = %d, sleeping = n/a", mutex.get_buffer()->is_locked(),
-                     mutex.get_buffer()->owner_ != nullptr
+                     mutex.get_buffer()->get_owner() != nullptr
                          ? (int)mc_model_checker->process()
-                               .resolve_actor(simgrid::mc::remote(mutex.get_buffer()->owner_))
+                               .resolve_actor(simgrid::mc::remote(mutex.get_buffer()->get_owner()))
                                ->get_pid()
                          : -1);
       break;

diff --git a/src/s4u/s4u_ConditionVariable.cpp b/src/s4u/s4u_ConditionVariable.cpp
index d55e0eb..c95fc4f 100644 (file)
--- a/src/s4u/s4u_ConditionVariable.cpp
+++ b/src/s4u/s4u_ConditionVariable.cpp
@@ -20,7 +20,7 @@ ConditionVariablePtr ConditionVariable::create()
 {
   kernel::activity::ConditionVariableImpl* cond =
       kernel::actor::simcall([] { return new kernel::activity::ConditionVariableImpl(); });
-  return ConditionVariablePtr(&cond->cond_, false);
+  return ConditionVariablePtr(cond->get_iface(), false);
 }
 
 /**

diff --git a/src/s4u/s4u_Semaphore.cpp b/src/s4u/s4u_Semaphore.cpp
index 1731d08..b4a5ed7 100644 (file)
--- a/src/s4u/s4u_Semaphore.cpp
+++ b/src/s4u/s4u_Semaphore.cpp
@@ -21,7 +21,7 @@ Semaphore::Semaphore(unsigned int initial_capacity)
 Semaphore::~Semaphore()
 {
   if (sem_ != nullptr) {
-    xbt_assert(sem_->sleeping_.empty(), "Cannot destroy semaphore since someone is still using it");
+    xbt_assert(not sem_->is_used(), "Cannot destroy semaphore since someone is still using it");
     delete sem_;
   }
 }

diff --git a/src/xbt/xbt_os_synchro.cpp b/src/xbt/xbt_os_synchro.cpp
index ecfe134..65829c9 100644 (file)
--- a/src/xbt/xbt_os_synchro.cpp
+++ b/src/xbt/xbt_os_synchro.cpp
@@ -57,12 +57,12 @@ int xbt_cond_timedwait(xbt_cond_t cond, xbt_mutex_t mutex, double delay)
 
 void xbt_cond_signal(xbt_cond_t cond)
 {
-  cond->cond_.notify_one();
+  cond->get_iface()->notify_one();
 }
 
 void xbt_cond_broadcast(xbt_cond_t cond)
 {
-  cond->cond_.notify_all();
+  cond->get_iface()->notify_all();
 }
 
 void xbt_cond_destroy(xbt_cond_t cond)