Cosmetics

- Rename xbt_main.cpp -> xbt_misc.cpp to reflect its content
- Stick to our naming schema (Engine::pimpl -> Engine::pimpl_)
- Don't say the loop iterator is const to then const_cast it
- Kill the leftover xbt::binary_name and xbt::cmdline local vars

diff --git a/MANIFEST.in b/MANIFEST.in
index 89864b3..d0b59a9 100644 (file)
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -2491,7 +2491,7 @@ include src/xbt/unit-tests_main.cpp
 include src/xbt/xbt_log_appender_file.cpp
 include src/xbt/xbt_log_layout_format.cpp
 include src/xbt/xbt_log_layout_simple.cpp
-include src/xbt/xbt_main.cpp
+include src/xbt/xbt_misc.cpp
 include src/xbt/xbt_os_file.cpp
 include src/xbt/xbt_os_time.c
 include src/xbt/xbt_parse_units.cpp

diff --git a/include/simgrid/Exception.hpp b/include/simgrid/Exception.hpp
index 66dd7a5..5747ce1 100644 (file)
--- a/include/simgrid/Exception.hpp
+++ b/include/simgrid/Exception.hpp
@@ -3,7 +3,7 @@
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
-/* This file defines all possible exception that could occur in a SimGrid library. */
+/* This file defines all possible exceptions that could occur in a SimGrid library. */
 
 #ifndef SIMGRID_EXCEPTIONS_HPP
 #define SIMGRID_EXCEPTIONS_HPP

diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp
index 0c37740..c9fd475 100644 (file)
--- a/include/simgrid/s4u/Engine.hpp
+++ b/include/simgrid/s4u/Engine.hpp
@@ -200,7 +200,10 @@ public:
     return res;
   }
 
-  kernel::EngineImpl* get_impl() const { return pimpl; }
+  kernel::EngineImpl* get_impl() const
+  {
+    return pimpl_;
+  }
 
   /** Returns whether SimGrid was initialized yet -- mostly for internal use */
   static bool is_initialized();
@@ -256,7 +259,7 @@ private:
   static xbt::signal<void(void)> on_deadlock;
   static xbt::signal<void()> on_simulation_end;
 
-  kernel::EngineImpl* const pimpl;
+  kernel::EngineImpl* const pimpl_;
   static Engine* instance_;
   void initialize(int* argc, char** argv);
 };

diff --git a/include/xbt/sysdep.h b/include/xbt/sysdep.h
index a4800a1..e78e0bd 100644 (file)
--- a/include/xbt/sysdep.h
+++ b/include/xbt/sysdep.h
@@ -78,8 +78,7 @@ static XBT_ALWAYS_INLINE void *xbt_realloc(void *p, size_t s) {
   return res;
 }
 
-/** @brief like free
-    @hideinitializer */
+/** @brief like free */
 #define xbt_free(p) free(p) /*nothing specific to do here. A poor valgrind replacement? */
 
 #ifdef __cplusplus

diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index b84f543..ab1ea07 100644 (file)
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -102,8 +102,14 @@ public:
   const std::vector<resource::Model*>& get_all_models() const { return models_; }
 
   static bool has_instance() { return s4u::Engine::has_instance(); }
-  static EngineImpl* get_instance() { return s4u::Engine::get_instance()->pimpl; }
-  static EngineImpl* get_instance(int* argc, char** argv) { return s4u::Engine::get_instance(argc, argv)->pimpl; }
+  static EngineImpl* get_instance()
+  {
+    return s4u::Engine::get_instance()->pimpl_;
+  }
+  static EngineImpl* get_instance(int* argc, char** argv)
+  {
+    return s4u::Engine::get_instance(argc, argv)->pimpl_;
+  }
 
   actor::ActorCodeFactory get_function(const std::string& name)
   {

diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp
index 3d7c6cb..de66d41 100644 (file)
--- a/src/s4u/s4u_Engine.cpp
+++ b/src/s4u/s4u_Engine.cpp
@@ -42,20 +42,20 @@ void Engine::initialize(int* argc, char** argv)
   xbt_assert(Engine::instance_ == nullptr, "It is currently forbidden to create more than one instance of s4u::Engine");
   Engine::instance_ = this;
   instr::init();
-  pimpl->initialize(argc, argv);
+  pimpl_->initialize(argc, argv);
   // Either create a new context with maestro or create
   // a context object with the current context maestro):
   kernel::actor::create_maestro(maestro_code);
 }
 
-Engine::Engine(std::string name) : pimpl(new kernel::EngineImpl())
+Engine::Engine(std::string name) : pimpl_(new kernel::EngineImpl())
 {
   int argc   = 1;
   char* argv = &name[0];
   initialize(&argc, &argv);
 }
 
-Engine::Engine(int* argc, char** argv) : pimpl(new kernel::EngineImpl())
+Engine::Engine(int* argc, char** argv) : pimpl_(new kernel::EngineImpl())
 {
   initialize(argc, argv);
 }
@@ -83,7 +83,7 @@ Engine* Engine::get_instance(int* argc, char** argv)
 }
 const std::vector<std::string>& Engine::get_cmdline() const
 {
-  return pimpl->get_cmdline();
+  return pimpl_->get_cmdline();
 }
 
 void Engine::shutdown() // XBT_ATTRIB_DEPRECATED_v335
@@ -103,22 +103,22 @@ double Engine::get_clock()
 void Engine::add_model(std::shared_ptr<kernel::resource::Model> model,
                        const std::vector<kernel::resource::Model*>& dependencies)
 {
-  kernel::actor::simcall_answered([this, &model, &dependencies] { pimpl->add_model(std::move(model), dependencies); });
+  kernel::actor::simcall_answered([this, &model, &dependencies] { pimpl_->add_model(std::move(model), dependencies); });
 }
 
 const std::vector<simgrid::kernel::resource::Model*>& Engine::get_all_models() const
 {
-  return pimpl->get_all_models();
+  return pimpl_->get_all_models();
 }
 
 void Engine::load_platform(const std::string& platf) const
 {
-  pimpl->load_platform(platf);
+  pimpl_->load_platform(platf);
 }
 
 void Engine::seal_platform() const
 {
-  pimpl->seal_platform();
+  pimpl_->seal_platform();
 }
 
 static void flatify_hosts(Engine const& engine, std::stringstream& ss)
@@ -287,12 +287,12 @@ void Engine::register_default(const std::function<void(int, char**)>& code)
 }
 void Engine::register_default(const kernel::actor::ActorCodeFactory& code)
 {
-  simgrid::kernel::actor::simcall_answered([this, &code]() { pimpl->register_default(code); });
+  simgrid::kernel::actor::simcall_answered([this, &code]() { pimpl_->register_default(code); });
 }
 
 void Engine::register_function(const std::string& name, const kernel::actor::ActorCodeFactory& code)
 {
-  simgrid::kernel::actor::simcall_answered([this, name, &code]() { pimpl->register_function(name, code); });
+  simgrid::kernel::actor::simcall_answered([this, name, &code]() { pimpl_->register_function(name, code); });
 }
 
 /** Load a deployment file and launch the actors that it contains
@@ -303,7 +303,7 @@ void Engine::register_function(const std::string& name, const kernel::actor::Act
  */
 void Engine::load_deployment(const std::string& deploy) const
 {
-  pimpl->load_deployment(deploy);
+  pimpl_->load_deployment(deploy);
 }
 
 /** Returns the amount of hosts in the platform */
@@ -320,8 +320,8 @@ std::vector<Host*> Engine::get_all_hosts() const
 std::vector<Host*> Engine::get_filtered_hosts(const std::function<bool(Host*)>& filter) const
 {
   std::vector<Host*> hosts;
-  if (pimpl->netzone_root_) {
-    hosts = pimpl->netzone_root_->get_filtered_hosts(filter);
+  if (pimpl_->netzone_root_) {
+    hosts = pimpl_->netzone_root_->get_filtered_hosts(filter);
   }
   /* Sort hosts in lexicographical order: keep same behavior when the hosts were saved on Engine
    * Some tests do a get_all_hosts() and selects hosts in this order */
@@ -346,8 +346,8 @@ Host* Engine::host_by_name(const std::string& name) const
 Host* Engine::host_by_name_or_null(const std::string& name) const
 {
   Host* host = nullptr;
-  if (pimpl->netzone_root_) {
-    auto* host_impl = pimpl->netzone_root_->get_host_by_name_or_null(name);
+  if (pimpl_->netzone_root_) {
+    auto* host_impl = pimpl_->netzone_root_->get_host_by_name_or_null(name);
     if (host_impl)
       host = host_impl->get_iface();
   }
@@ -368,7 +368,8 @@ Link* Engine::link_by_name(const std::string& name) const
 
 SplitDuplexLink* Engine::split_duplex_link_by_name(const std::string& name) const
 {
-  auto* link_impl = pimpl->netzone_root_ ? pimpl->netzone_root_->get_split_duplex_link_by_name_or_null(name) : nullptr;
+  auto* link_impl =
+      pimpl_->netzone_root_ ? pimpl_->netzone_root_->get_split_duplex_link_by_name_or_null(name) : nullptr;
   if (not link_impl)
     throw std::invalid_argument("Link not found: " + name);
   return link_impl->get_iface();
@@ -378,11 +379,11 @@ SplitDuplexLink* Engine::split_duplex_link_by_name(const std::string& name) cons
 Link* Engine::link_by_name_or_null(const std::string& name) const
 {
   Link* link = nullptr;
-  if (pimpl->netzone_root_) {
+  if (pimpl_->netzone_root_) {
     /* keep behavior where internal __loopback__ link from network model is given to user */
     if (name == "__loopback__")
-      return pimpl->netzone_root_->get_network_model()->loopback_->get_iface();
-    auto* link_impl = pimpl->netzone_root_->get_link_by_name_or_null(name);
+      return pimpl_->netzone_root_->get_network_model()->loopback_->get_iface();
+    auto* link_impl = pimpl_->netzone_root_->get_link_by_name_or_null(name);
     if (link_impl)
       link = link_impl->get_iface();
   }
@@ -394,7 +395,7 @@ Mailbox* Engine::mailbox_by_name_or_create(const std::string& name) const
 {
   /* two actors may have pushed the same mbox_create simcall at the same time */
   kernel::activity::MailboxImpl* mbox = kernel::actor::simcall_answered([&name, this] {
-    auto [m, inserted] = pimpl->mailboxes_.try_emplace(name, nullptr);
+    auto [m, inserted] = pimpl_->mailboxes_.try_emplace(name, nullptr);
     if (inserted) {
       m->second = new kernel::activity::MailboxImpl(name);
       XBT_DEBUG("Creating a mailbox at %p with name %s", m->second, name.c_str());
@@ -408,10 +409,10 @@ Mailbox* Engine::mailbox_by_name_or_create(const std::string& name) const
 size_t Engine::get_link_count() const
 {
   int count = 0;
-  if (pimpl->netzone_root_) {
-    count += pimpl->netzone_root_->get_link_count();
+  if (pimpl_->netzone_root_) {
+    count += pimpl_->netzone_root_->get_link_count();
     /* keep behavior where internal __loopback__ link from network model is given to user */
-    count += pimpl->netzone_root_->get_network_model()->loopback_ ? 1 : 0;
+    count += pimpl_->netzone_root_->get_network_model()->loopback_ ? 1 : 0;
   }
   return count;
 }
@@ -425,25 +426,25 @@ std::vector<Link*> Engine::get_all_links() const
 std::vector<Link*> Engine::get_filtered_links(const std::function<bool(Link*)>& filter) const
 {
   std::vector<Link*> res;
-  if (pimpl->netzone_root_) {
-    res = pimpl->netzone_root_->get_filtered_links(filter);
+  if (pimpl_->netzone_root_) {
+    res = pimpl_->netzone_root_->get_filtered_links(filter);
     /* keep behavior where internal __loopback__ link from network model is given to user */
-    if (pimpl->netzone_root_->get_network_model()->loopback_ &&
-        filter(pimpl->netzone_root_->get_network_model()->loopback_->get_iface()))
-      res.push_back(pimpl->netzone_root_->get_network_model()->loopback_->get_iface());
+    if (pimpl_->netzone_root_->get_network_model()->loopback_ &&
+        filter(pimpl_->netzone_root_->get_network_model()->loopback_->get_iface()))
+      res.push_back(pimpl_->netzone_root_->get_network_model()->loopback_->get_iface());
   }
   return res;
 }
 
 size_t Engine::get_actor_count() const
 {
-  return pimpl->get_actor_count();
+  return pimpl_->get_actor_count();
 }
 
 std::vector<ActorPtr> Engine::get_all_actors() const
 {
   std::vector<ActorPtr> actor_list;
-  for (auto const& [_, actor] : pimpl->get_actor_list()) {
+  for (auto const& [_, actor] : pimpl_->get_actor_list()) {
     actor_list.push_back(actor->get_iface());
   }
   return actor_list;
@@ -452,7 +453,7 @@ std::vector<ActorPtr> Engine::get_all_actors() const
 std::vector<ActorPtr> Engine::get_filtered_actors(const std::function<bool(ActorPtr)>& filter) const
 {
   std::vector<ActorPtr> actor_list;
-  for (auto const& [_, actor] : pimpl->get_actor_list()) {
+  for (auto const& [_, actor] : pimpl_->get_actor_list()) {
     if (filter(actor->get_iface()))
       actor_list.push_back(actor->get_iface());
   }
@@ -473,7 +474,7 @@ void Engine::run_until(double max_date) const
   fflush(stdout);
   fflush(stderr);
 
-  pimpl->run(max_date);
+  pimpl_->run(max_date);
 }
 
 void Engine::track_vetoed_activities(std::set<Activity*>* vetoed_activities) const
@@ -484,15 +485,15 @@ void Engine::track_vetoed_activities(std::set<Activity*>* vetoed_activities) con
 /** @brief Retrieve the root netzone, containing all others */
 s4u::NetZone* Engine::get_netzone_root() const
 {
-  if (pimpl->netzone_root_)
-    return pimpl->netzone_root_->get_iface();
+  if (pimpl_->netzone_root_)
+    return pimpl_->netzone_root_->get_iface();
   return nullptr;
 }
 /** @brief Set the root netzone, containing all others. Once set, it cannot be changed. */
 void Engine::set_netzone_root(const s4u::NetZone* netzone)
 {
-  xbt_assert(pimpl->netzone_root_ == nullptr, "The root NetZone cannot be changed once set");
-  pimpl->netzone_root_ = netzone->get_impl();
+  xbt_assert(pimpl_->netzone_root_ == nullptr, "The root NetZone cannot be changed once set");
+  pimpl_->netzone_root_ = netzone->get_impl();
 }
 
 static NetZone* netzone_by_name_recursive(NetZone* current, const std::string& name)
@@ -518,8 +519,8 @@ NetZone* Engine::netzone_by_name_or_null(const std::string& name) const
 /** @brief Retrieve the netpoint of the given name (or nullptr if not found) */
 kernel::routing::NetPoint* Engine::netpoint_by_name_or_null(const std::string& name) const
 {
-  auto netp = pimpl->netpoints_.find(name);
-  return netp == pimpl->netpoints_.end() ? nullptr : netp->second;
+  auto netp = pimpl_->netpoints_.find(name);
+  return netp == pimpl_->netpoints_.end() ? nullptr : netp->second;
 }
 
 kernel::routing::NetPoint* Engine::netpoint_by_name(const std::string& name) const
@@ -534,7 +535,7 @@ kernel::routing::NetPoint* Engine::netpoint_by_name(const std::string& name) con
 std::vector<kernel::routing::NetPoint*> Engine::get_all_netpoints() const
 {
   std::vector<kernel::routing::NetPoint*> res;
-  for (auto const& [_, netpoint] : pimpl->netpoints_)
+  for (auto const& [_, netpoint] : pimpl_->netpoints_)
     res.push_back(netpoint);
   return res;
 }
@@ -542,14 +543,14 @@ std::vector<kernel::routing::NetPoint*> Engine::get_all_netpoints() const
 /** @brief Register a new netpoint to the system */
 void Engine::netpoint_register(kernel::routing::NetPoint* point)
 {
-  simgrid::kernel::actor::simcall_answered([this, point] { pimpl->netpoints_[point->get_name()] = point; });
+  simgrid::kernel::actor::simcall_answered([this, point] { pimpl_->netpoints_[point->get_name()] = point; });
 }
 
 /** @brief Unregister a given netpoint */
 void Engine::netpoint_unregister(kernel::routing::NetPoint* point)
 {
   kernel::actor::simcall_answered([this, point] {
-    pimpl->netpoints_.erase(point->get_name());
+    pimpl_->netpoints_.erase(point->get_name());
     delete point;
   });
 }

diff --git a/src/xbt/log.cpp b/src/xbt/log.cpp
index d546f82..65c5dae 100644 (file)
--- a/src/xbt/log.cpp
+++ b/src/xbt/log.cpp
@@ -113,8 +113,8 @@ static void log_cat_exit(xbt_log_category_t cat)
     cat->layout = nullptr;
   }
 
-  for (auto const* child = cat->firstChild; child != nullptr; child = child->nextSibling)
-    log_cat_exit(const_cast<xbt_log_category_t>(child));
+  for (auto* child = cat->firstChild; child != nullptr; child = child->nextSibling)
+    log_cat_exit(child);
   cat->firstChild = nullptr;
 }
 

diff --git a/src/xbt/xbt_main.cpp b/src/xbt/xbt_misc.cpp
similarity index 69%
rename from src/xbt/xbt_main.cpp
rename to src/xbt/xbt_misc.cpp
index 8c9b9c7..d5522f8 100644 (file)
--- a/src/xbt/xbt_main.cpp
+++ b/src/xbt/xbt_misc.cpp
@@ -1,4 +1,4 @@
-/* module handling                                                          */
+/* Various pieces of code which don't fit in any module                     */
 
 /* Copyright (c) 2006-2023. The SimGrid Team. All rights reserved.          */
 
@@ -7,34 +7,21 @@
 
 #define XBT_LOG_LOCALLY_DEFINE_XBT_CHANNEL /* MSVC don't want it to be declared extern in headers and local here */
 
-#include "simgrid/config.h"
 #include "src/internal_config.h"
-#include "src/simgrid/sg_config.hpp"
 #include "src/sthread/sthread.h" // sthread_inside_simgrid
 #include "src/xbt/coverage.h"
-#include "xbt/config.hpp"
-#include "xbt/dynar.h"
 #include "xbt/log.h"
-#include "xbt/log.hpp"
 #include "xbt/misc.h"
 #include "xbt/sysdep.h"
 
 #include <cmath>
-#include <cstdio>
 #if HAVE_UNISTD_H
-# include <unistd.h>
+#include <unistd.h>
 #endif
-#include <string>
-#include <vector>
 
-XBT_LOG_NEW_DEFAULT_SUBCATEGORY(module, xbt, "module handling");
-
-XBT_LOG_NEW_CATEGORY(smpi, "All SMPI categories"); /* lives here even if that's a bit odd to solve linking issues: this is used in xbt_log_file_appender to detect whether SMPI is used (and thus whether we should unbench the writing to disk) */
-
-namespace simgrid::xbt {
-std::string binary_name;          /* Name of the system process containing us (mandatory to retrieve neat backtraces) */
-std::vector<std::string> cmdline; /* all we got in argv */
-} // namespace simgrid::xbt
+XBT_LOG_NEW_CATEGORY(smpi, "All SMPI categories"); /* lives here even if that's a bit odd to solve linking issues: this
+                                                      is used in xbt_log_file_appender to detect whether SMPI is used
+                                                      (and thus whether we should unbench the writing to disk) */
 
 const int xbt_pagesize = static_cast<int>(sysconf(_SC_PAGESIZE));
 const int xbt_pagebits = static_cast<int>(log2(xbt_pagesize));

diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake
index 9776b8c..58af6c4 100644 (file)
--- a/tools/cmake/DefinePackages.cmake
+++ b/tools/cmake/DefinePackages.cmake
@@ -277,7 +277,7 @@ set(XBT_SRC
   src/xbt/xbt_log_appender_file.cpp
   src/xbt/xbt_log_layout_format.cpp
   src/xbt/xbt_log_layout_simple.cpp
-  src/xbt/xbt_main.cpp
+  src/xbt/xbt_misc.cpp
   src/xbt/xbt_os_file.cpp
   src/xbt/xbt_os_time.c
   src/xbt/xbt_parse_units.cpp