From aefe6fe5db48f4b71f9c4d69daf6ac7ebe2fbb01 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Tue, 21 Feb 2023 22:08:45 +0100 Subject: [PATCH] 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 --- MANIFEST.in | 2 +- include/simgrid/Exception.hpp | 2 +- include/simgrid/s4u/Engine.hpp | 7 ++- include/xbt/sysdep.h | 3 +- src/kernel/EngineImpl.hpp | 10 +++- src/s4u/s4u_Engine.cpp | 83 +++++++++++++------------- src/xbt/log.cpp | 4 +- src/xbt/{xbt_main.cpp => xbt_misc.cpp} | 23 ++----- tools/cmake/DefinePackages.cmake | 2 +- 9 files changed, 66 insertions(+), 70 deletions(-) rename src/xbt/{xbt_main.cpp => xbt_misc.cpp} (69%) diff --git a/MANIFEST.in b/MANIFEST.in index 89864b30ac..d0b59a996a 100644 --- 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 66dd7a5ee3..5747ce1f44 100644 --- 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 0c3774068d..c9fd4754f4 100644 --- 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 on_deadlock; static xbt::signal 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 a4800a1e05..e78e0bd656 100644 --- 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 b84f5430da..ab1ea0711f 100644 --- a/src/kernel/EngineImpl.hpp +++ b/src/kernel/EngineImpl.hpp @@ -102,8 +102,14 @@ public: const std::vector& 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 3d7c6cbe82..de66d412c8 100644 --- 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& 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 model, const std::vector& 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& 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& 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 Engine::get_all_hosts() const std::vector Engine::get_filtered_hosts(const std::function& filter) const { std::vector 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 Engine::get_all_links() const std::vector Engine::get_filtered_links(const std::function& filter) const { std::vector 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 Engine::get_all_actors() const { std::vector 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 Engine::get_all_actors() const std::vector Engine::get_filtered_actors(const std::function& filter) const { std::vector 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* vetoed_activities) const @@ -484,15 +485,15 @@ void Engine::track_vetoed_activities(std::set* 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 Engine::get_all_netpoints() const { std::vector 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 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 d546f82e8d..65c5dae424 100644 --- 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(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 8c9b9c75c2..d5522f8a54 100644 --- 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 -#include #if HAVE_UNISTD_H -# include +#include #endif -#include -#include -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 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(sysconf(_SC_PAGESIZE)); const int xbt_pagebits = static_cast(log2(xbt_pagesize)); diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake index 9776b8c3d2..58af6c463d 100644 --- 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 -- 2.20.1