Fix some easy sonar smells

const-ness, try_emplace, attribute noreturn, ...

The most important one is the TransitionObjectAccess one, where a
field in the subclass was hiding a field of the same name in the
superclass. Maybe the bug I was experiencing in that area was related.

diff --git a/src/mc/api/RemoteApp.cpp b/src/mc/api/RemoteApp.cpp
index 114c7e4..45e595e 100644 (file)
--- a/src/mc/api/RemoteApp.cpp
+++ b/src/mc/api/RemoteApp.cpp
@@ -185,8 +185,7 @@ Transition* RemoteApp::handle_simcall(aid_t aid, int times_considered, bool new_
   m.times_considered_              = times_considered;
   checker_side_->get_channel().send(m);
 
-  auto* memory = get_remote_process_memory();
-  if (memory != nullptr)
+  if (auto* memory = get_remote_process_memory(); memory != nullptr)
     memory->clear_cache();
   if (checker_side_->running())
     checker_side_->dispatch_events(); // The app may send messages while processing the transition

diff --git a/src/mc/api/State.cpp b/src/mc/api/State.cpp
index b779fdc..967c625 100644 (file)
--- a/src/mc/api/State.cpp
+++ b/src/mc/api/State.cpp
@@ -46,7 +46,7 @@ State::State(RemoteApp& remote_app, const State* parent_state)
 
       if (not parent_state_->get_transition()->depends(&transition)) {
 
-        sleep_set_.emplace(aid, transition);
+        sleep_set_.try_emplace(aid, transition);
         if (guide->actors_to_run_.count(aid) != 0) {
           XBT_DEBUG("Actor %ld will not be explored, for it is in the sleep set", aid);
 

diff --git a/src/mc/api/State.hpp b/src/mc/api/State.hpp
index 8626d99..1dd6ecc 100644 (file)
--- a/src/mc/api/State.hpp
+++ b/src/mc/api/State.hpp
@@ -83,7 +83,7 @@ public:
   void set_system_state(std::shared_ptr<Snapshot> state) { system_state_ = std::move(state); }
 
   std::map<aid_t, Transition> const& get_sleep_set() const { return sleep_set_; }
-  void add_sleep_set(Transition* t)
+  void add_sleep_set(const Transition* t)
   {
     sleep_set_.insert_or_assign(t->aid_, Transition(t->type_, t->aid_, t->times_considered_));
   }

diff --git a/src/mc/explo/Exploration.cpp b/src/mc/explo/Exploration.cpp
index 6be979e..1f0e24f 100644 (file)
--- a/src/mc/explo/Exploration.cpp
+++ b/src/mc/explo/Exploration.cpp
@@ -66,7 +66,7 @@ void Exploration::log_state()
   }
 }
 
-void Exploration::report_crash(int status)
+XBT_ATTRIB_NORETURN void Exploration::report_crash(int status)
 {
   XBT_INFO("**************************");
   XBT_INFO("** CRASH IN THE PROGRAM **");
@@ -88,7 +88,7 @@ void Exploration::report_crash(int status)
   if (xbt_log_no_loc) {
     XBT_INFO("Stack trace not displayed because you passed --log=no_loc");
   } else {
-    auto* memory = get_remote_app().get_remote_process_memory();
+    const auto* memory = get_remote_app().get_remote_process_memory();
     if (memory) {
       XBT_INFO("Stack trace:");
       memory->dump_stack();
@@ -99,7 +99,7 @@ void Exploration::report_crash(int status)
 
   system_exit(SIMGRID_MC_EXIT_PROGRAM_CRASH);
 }
-void Exploration::report_assertion_failure()
+XBT_ATTRIB_NORETURN void Exploration::report_assertion_failure()
 {
   XBT_INFO("**************************");
   XBT_INFO("*** PROPERTY NOT VALID ***");
@@ -114,7 +114,7 @@ void Exploration::report_assertion_failure()
   system_exit(SIMGRID_MC_EXIT_SAFETY);
 }
 
-void Exploration::system_exit(int status)
+void Exploration::system_exit(int status) const
 {
   ::exit(status);
 }

diff --git a/src/mc/explo/Exploration.hpp b/src/mc/explo/Exploration.hpp
index 7a30470..aa6d420 100644 (file)
--- a/src/mc/explo/Exploration.hpp
+++ b/src/mc/explo/Exploration.hpp
@@ -46,12 +46,12 @@ public:
   virtual void run() = 0;
 
   /** Produce an error message indicating that the application crashed (status was produced by waitpid) */
-  void report_crash(int status);
+  XBT_ATTRIB_NORETURN void report_crash(int status);
   /** Produce an error message indicating that a property was violated */
-  void report_assertion_failure();
+  XBT_ATTRIB_NORETURN void report_assertion_failure();
 
   /** Kill the application and the model-checker (which exits with `status`)*/
-  XBT_ATTRIB_NORETURN void system_exit(int status);
+  XBT_ATTRIB_NORETURN void system_exit(int status) const;
 
   /* These methods are callbacks called by the model-checking engine
    * to get and display information about the current state of the

diff --git a/src/mc/remote/CheckerSide.cpp b/src/mc/remote/CheckerSide.cpp
index 92d63d4..5958cd3 100644 (file)
--- a/src/mc/remote/CheckerSide.cpp
+++ b/src/mc/remote/CheckerSide.cpp
@@ -133,7 +133,7 @@ void CheckerSide::setup_events()
 
   socket_event_ = event_new(
       base, get_channel().get_socket(), EV_READ | EV_PERSIST,
-      [](evutil_socket_t sig, short events, void* arg) {
+      [](evutil_socket_t, short events, void* arg) {
         auto checker = static_cast<simgrid::mc::CheckerSide*>(arg);
         if (events == EV_READ) {
           std::array<char, MC_MESSAGE_LENGTH> buffer;
@@ -283,7 +283,7 @@ bool CheckerSide::handle_message(const char* buffer, ssize_t size)
         s_mc_message_ignore_memory_t message;
         xbt_assert(size == sizeof(message), "Broken message");
         memcpy(&message, buffer, sizeof(message));
-        get_remote_memory()->unignore_heap((void*)(std::uintptr_t)message.addr, message.size);
+        get_remote_memory()->unignore_heap((void*)message.addr, message.size);
       } else {
         XBT_INFO("Ignoring an UNIGNORE_HEAP message because we don't need to introspect memory.");
       }

diff --git a/src/mc/sosp/Region.cpp b/src/mc/sosp/Region.cpp
index acbc080..2dca797 100644 (file)
--- a/src/mc/sosp/Region.cpp
+++ b/src/mc/sosp/Region.cpp
@@ -16,7 +16,8 @@
 
 namespace simgrid::mc {
 
-Region::Region(PageStore& store, RemoteProcessMemory& memory, RegionType region_type, void* start_addr, size_t size)
+Region::Region(PageStore& store, const RemoteProcessMemory& memory, RegionType region_type, void* start_addr,
+               size_t size)
     : region_type_(region_type), start_addr_(start_addr), size_(size)
 {
   xbt_assert((((uintptr_t)start_addr) & (xbt_pagesize - 1)) == 0, "Start address not at the beginning of a page");
@@ -28,7 +29,7 @@ Region::Region(PageStore& store, RemoteProcessMemory& memory, RegionType region_
  *
  *  @param region     Target region
  */
-void Region::restore(RemoteProcessMemory& memory) const
+void Region::restore(const RemoteProcessMemory& memory) const
 {
   xbt_assert(((start().address()) & (xbt_pagesize - 1)) == 0, "Not at the beginning of a page");
   xbt_assert(simgrid::mc::mmu::chunk_count(size()) == get_chunks().page_count());

diff --git a/src/mc/sosp/Region.hpp b/src/mc/sosp/Region.hpp
index cf7b748..7ab26e2 100644 (file)
--- a/src/mc/sosp/Region.hpp
+++ b/src/mc/sosp/Region.hpp
@@ -35,7 +35,7 @@ private:
   ChunkedData chunks_;
 
 public:
-  Region(PageStore& store, RemoteProcessMemory& memory, RegionType type, void* start_addr, size_t size);
+  Region(PageStore& store, const RemoteProcessMemory& memory, RegionType type, void* start_addr, size_t size);
   Region(Region const&) = delete;
   Region& operator=(Region const&) = delete;
   Region(Region&& that)            = delete;
@@ -58,7 +58,7 @@ public:
   bool contain(RemotePtr<void> p) const { return p >= start() && p < end(); }
 
   /** @brief Restore a region from a snapshot */
-  void restore(RemoteProcessMemory& memory) const;
+  void restore(const RemoteProcessMemory& memory) const;
 
   /** @brief Read memory that was snapshotted in this region
    *

diff --git a/src/mc/sosp/Snapshot_test.cpp b/src/mc/sosp/Snapshot_test.cpp
index 237065b..d2f7d0e 100644 (file)
--- a/src/mc/sosp/Snapshot_test.cpp
+++ b/src/mc/sosp/Snapshot_test.cpp
@@ -151,8 +151,8 @@ void snap_test_helper::compare_region_parts()
   }
 }
 
-int some_global_variable  = 42;
-void* some_global_pointer = &some_global_variable;
+const int some_global_variable  = 42;
+const void* some_global_pointer = &some_global_variable;
 void snap_test_helper::read_pointer()
 {
   prologue_return ret = prologue(1);

diff --git a/src/mc/transition/TransitionObjectAccess.cpp b/src/mc/transition/TransitionObjectAccess.cpp
index d16472e..63a7d92 100644 (file)
--- a/src/mc/transition/TransitionObjectAccess.cpp
+++ b/src/mc/transition/TransitionObjectAccess.cpp
@@ -15,14 +15,14 @@ ObjectAccessTransition::ObjectAccessTransition(aid_t issuer, int times_considere
 {
   short s;
   xbt_assert(stream >> s >> objaddr_ >> objname_ >> file_ >> line_);
-  type_ = static_cast<simgrid::mc::ObjectAccessType>(s);
+  access_type_ = static_cast<simgrid::mc::ObjectAccessType>(s);
 }
 std::string ObjectAccessTransition::to_string(bool verbose) const
 {
   std::string res;
-  if (type_ == ObjectAccessType::ENTER)
+  if (access_type_ == ObjectAccessType::ENTER)
     res = std::string("BeginObjectAccess(");
-  else if (type_ == ObjectAccessType::EXIT)
+  else if (access_type_ == ObjectAccessType::EXIT)
     res = std::string("EndObjectAccess(");
   else
     res = std::string("ObjectAccess(");

diff --git a/src/mc/transition/TransitionObjectAccess.hpp b/src/mc/transition/TransitionObjectAccess.hpp
index 1231446..4ca7ff8 100644 (file)
--- a/src/mc/transition/TransitionObjectAccess.hpp
+++ b/src/mc/transition/TransitionObjectAccess.hpp
@@ -12,7 +12,7 @@ namespace simgrid::mc {
 XBT_DECLARE_ENUM_CLASS(ObjectAccessType, ENTER, EXIT, BOTH);
 
 class ObjectAccessTransition : public Transition {
-  ObjectAccessType type_;
+  ObjectAccessType access_type_;
   void* objaddr_;
   std::string objname_;
   std::string file_;