From 6d1fc1c31cb2152b6d20742081118524dbb78d14 Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Fri, 19 May 2017 10:03:07 +0200 Subject: [PATCH 1/1] bugs, smells and cosmetics of the day --- include/simgrid/s4u/Comm.hpp | 2 +- include/xbt/ex.hpp | 2 +- src/plugins/vm/VmHostExt.cpp | 3 --- src/plugins/vm/VmHostExt.hpp | 2 +- src/s4u/s4u_comm.cpp | 6 ----- src/surf/surf_interface.cpp | 5 ++-- src/surf/xml/surfxml_sax_cb.cpp | 43 ++++++++++++++++++++++++--------- src/xbt/dict.cpp | 8 ++++-- src/xbt/ex.cpp | 8 +++--- 9 files changed, 45 insertions(+), 34 deletions(-) diff --git a/include/simgrid/s4u/Comm.hpp b/include/simgrid/s4u/Comm.hpp index 2a49c55e15..e2db122157 100644 --- a/include/simgrid/s4u/Comm.hpp +++ b/include/simgrid/s4u/Comm.hpp @@ -23,7 +23,7 @@ XBT_PUBLIC_CLASS Comm : public Activity { Comm() : Activity() {} public: - ~Comm() override; + virtual ~Comm() = default; /*! take a range of s4u::Comm* (last excluded) and return when one of them is finished. The return value is an * iterator on the finished Comms. */ diff --git a/include/xbt/ex.hpp b/include/xbt/ex.hpp index 4b940b83be..882f15b2d7 100644 --- a/include/xbt/ex.hpp +++ b/include/xbt/ex.hpp @@ -79,7 +79,7 @@ struct XBT_PUBLIC() xbt_ex : simgrid::xbt::WithContextException(throwpoint, simgrid::xbt::backtrace()) {} - ~xbt_ex() override; + virtual ~xbt_ex() = default; /** Category (what went wrong) */ xbt_errcat_t category = unknown_error; diff --git a/src/plugins/vm/VmHostExt.cpp b/src/plugins/vm/VmHostExt.cpp index 17d3e9ca4e..14d524b9a8 100644 --- a/src/plugins/vm/VmHostExt.cpp +++ b/src/plugins/vm/VmHostExt.cpp @@ -12,9 +12,6 @@ namespace simgrid { namespace vm { simgrid::xbt::Extension VmHostExt::EXTENSION_ID; -VmHostExt::~VmHostExt() -{ -} void VmHostExt::ensureVmExtInstalled() { if (!EXTENSION_ID.valid()) diff --git a/src/plugins/vm/VmHostExt.hpp b/src/plugins/vm/VmHostExt.hpp index 0a0bedbbf8..ee657b03c1 100644 --- a/src/plugins/vm/VmHostExt.hpp +++ b/src/plugins/vm/VmHostExt.hpp @@ -14,7 +14,7 @@ namespace vm { class VmHostExt { public: static simgrid::xbt::Extension EXTENSION_ID; - virtual ~VmHostExt(); + virtual ~VmHostExt() = default; sg_size_t ramsize = 0; /* available ramsize (0= not taken into account) */ bool overcommit = true; /* Whether the host allows overcommiting more VM than the avail ramsize allows */ diff --git a/src/s4u/s4u_comm.cpp b/src/s4u/s4u_comm.cpp index 5fecc9563d..9ecedf2a27 100644 --- a/src/s4u/s4u_comm.cpp +++ b/src/s4u/s4u_comm.cpp @@ -14,12 +14,6 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_comm,s4u_activity,"S4U asynchronous communic namespace simgrid { namespace s4u { -Comm::~Comm() { - -} - - - s4u::Comm &Comm::send_init(s4u::MailboxPtr chan) { s4u::Comm *res = new s4u::Comm(); res->sender_ = SIMIX_process_self(); diff --git a/src/surf/surf_interface.cpp b/src/surf/surf_interface.cpp index 2b2fa3ad51..7ef46e5423 100644 --- a/src/surf/surf_interface.cpp +++ b/src/surf/surf_interface.cpp @@ -482,20 +482,19 @@ double Model::nextOccuringEventLazy(double now) } if ((action->getMaxDuration() > NO_MAX_DURATION) && - (min == -1 || action->getStartTime() + action->getMaxDuration() < min)) { + (min <= -1 || action->getStartTime() + action->getMaxDuration() < min)) { // when the task will complete anyway because of the deadline if any min = action->getStartTime() + action->getMaxDuration(); max_dur_flag = true; } - XBT_DEBUG("Action(%p) corresponds to variable %d", action, action->getVariable()->id_int); XBT_DEBUG("Action(%p) Start %f. May finish at %f (got a share of %f). Max_duration %f", action, action->getStartTime(), min, share, action->getMaxDuration()); - if (min != -1) { + if (min > -1) { action->heapUpdate(actionHeap_, min, max_dur_flag ? MAX_DURATION : NORMAL); XBT_DEBUG("Insert at heap action(%p) min %f now %f", action, min, now); } else diff --git a/src/surf/xml/surfxml_sax_cb.cpp b/src/surf/xml/surfxml_sax_cb.cpp index d59eeb0523..03adf834bf 100644 --- a/src/surf/xml/surfxml_sax_cb.cpp +++ b/src/surf/xml/surfxml_sax_cb.cpp @@ -703,6 +703,9 @@ void STag_surfxml_link___ctn(){ link_name = bprintf("%s_DOWN", A_surfxml_link___ctn_id); link = simgrid::surf::LinkImpl::byName(link_name); break; + default: + surf_parse_error("Invalid direction for link %s", link_name); + break; } xbt_free(link_name); // no-op if it's already nullptr @@ -911,7 +914,8 @@ void ETag_surfxml_trace(){ sg_platf_new_trace(&trace); } -void STag_surfxml_trace___connect(){ +void STag_surfxml_trace___connect() +{ parse_after_config(); s_sg_platf_trace_connect_cbarg_t trace_connect; memset(&trace_connect,0,sizeof(trace_connect)); @@ -936,30 +940,41 @@ void STag_surfxml_trace___connect(){ case A_surfxml_trace___connect_kind_LINK___AVAIL: trace_connect.kind = SURF_TRACE_CONNECT_KIND_LINK_AVAIL; break; + default: + surf_parse_error("Invalid trace kind"); + break; } sg_platf_trace_connect(&trace_connect); } -void STag_surfxml_AS(){ +void STag_surfxml_AS() +{ AX_surfxml_zone_id = AX_surfxml_AS_id; AX_surfxml_zone_routing = (AT_surfxml_zone_routing)AX_surfxml_AS_routing; STag_surfxml_zone(); } -void ETag_surfxml_AS(){ + +void ETag_surfxml_AS() +{ ETag_surfxml_zone(); } -void STag_surfxml_zone(){ + +void STag_surfxml_zone() +{ parse_after_config(); ZONE_TAG = 1; - s_sg_platf_AS_cbarg_t AS = { A_surfxml_zone_id, (int)A_surfxml_zone_routing}; + s_sg_platf_AS_cbarg_t AS = {A_surfxml_zone_id, (int)A_surfxml_zone_routing}; sg_platf_new_AS_begin(&AS); } -void ETag_surfxml_zone(){ + +void ETag_surfxml_zone() +{ sg_platf_new_AS_seal(); } -void STag_surfxml_config(){ +void STag_surfxml_config() +{ ZONE_TAG = 0; xbt_assert(current_property_set == nullptr, "Someone forgot to reset the property set to nullptr in its closing tag (or XML malformed)"); XBT_DEBUG("START configuration name = %s",A_surfxml_config_id); @@ -968,15 +983,16 @@ void STag_surfxml_config(){ ", etc)."); } } -void ETag_surfxml_config(){ + +void ETag_surfxml_config() +{ xbt_dict_cursor_t cursor = nullptr; char *key; char *elem; xbt_dict_foreach(current_property_set, cursor, key, elem) { if (xbt_cfg_is_default_value(key)) { - char* cfg = bprintf("%s:%s", key, elem); - xbt_cfg_set_parse(cfg); - free(cfg); + std::string cfg = std::string(key) + ":" + elem; + xbt_cfg_set_parse(cfg.c_str()); } else XBT_INFO("The custom configuration '%s' is already defined by user!",key); } @@ -1033,6 +1049,9 @@ void ETag_surfxml_actor() case A_surfxml_actor_on___failure_RESTART: actor.on_failure = SURF_ACTOR_ON_FAILURE_RESTART; break; + default: + surf_parse_error("Invalid on failure behavior"); + break; } sg_platf_new_process(&actor); @@ -1111,6 +1130,6 @@ static int _surf_parse() { return surf_parse_lex(); } -int_f_void_t surf_parse = _surf_parse; +int_f_void_t surf_parse = &_surf_parse; SG_END_DECL() diff --git a/src/xbt/dict.cpp b/src/xbt/dict.cpp index cc112b9f49..9d5dc32464 100644 --- a/src/xbt/dict.cpp +++ b/src/xbt/dict.cpp @@ -113,7 +113,8 @@ static void xbt_dict_rehash(xbt_dict_t dict) xbt_dictelm_t *currcell = (xbt_dictelm_t *) xbt_realloc((char *) dict->table, newsize * sizeof(xbt_dictelm_t)); memset(&currcell[oldsize], 0, oldsize * sizeof(xbt_dictelm_t)); /* zero second half */ - dict->table_size = --newsize; + newsize--; + dict->table_size = newsize; dict->table = currcell; XBT_DEBUG("REHASH (%d->%d)", oldsize, newsize); @@ -559,7 +560,10 @@ void xbt_dict_postexit() total_count += size; } } - printf("; %f elm per cell\n", avg / (double) total_count); + if (total_count > 0) + printf("; %f elm per cell\n", avg / (double)total_count); + else + printf("; 0 elm per cell\n"); } } diff --git a/src/xbt/ex.cpp b/src/xbt/ex.cpp index f6db218437..b0f5564aa7 100644 --- a/src/xbt/ex.cpp +++ b/src/xbt/ex.cpp @@ -59,11 +59,7 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(xbt_ex, xbt, "Exception mechanism"); -xbt_ex::~xbt_ex() {} - -void _xbt_throw( - char* message, xbt_errcat_t errcat, int value, - const char* file, int line, const char* func) +void _xbt_throw(char* message, xbt_errcat_t errcat, int value, const char* file, int line, const char* func) { xbt_ex e(simgrid::xbt::ThrowPoint(file, line, func), message); free(message); @@ -110,6 +106,8 @@ const char *xbt_ex_catname(xbt_errcat_t cat) return "io error"; case vm_error: return "vm error"; + default: + return "INVALID ERROR"; } return "INVALID ERROR"; } -- 2.20.1