From: Frederic Suter Date: Thu, 23 Feb 2017 10:52:26 +0000 (+0100) Subject: smells and bugs X-Git-Tag: v3_15~307 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/16f92c056ae54aa6ddcd59522e469a74b48f30b7?hp=ed23ac2bff8fcad35229de335d7a42d7fa65d53e smells and bugs --- diff --git a/src/surf/PropertyHolder.cpp b/src/surf/PropertyHolder.cpp index 48f372fb52..0683cc37e3 100644 --- a/src/surf/PropertyHolder.cpp +++ b/src/surf/PropertyHolder.cpp @@ -19,7 +19,7 @@ PropertyHolder::~PropertyHolder() { const char *PropertyHolder::getProperty(const char*key) { if (properties_ == nullptr) return nullptr; - return (const char*) xbt_dict_get_or_null(properties_,key); + return static_cast(xbt_dict_get_or_null(properties_,key)); } /** @brief Change the value of a given key in the property set */ diff --git a/src/surf/fair_bottleneck.cpp b/src/surf/fair_bottleneck.cpp index 04a64dcfde..d6c424a8b3 100644 --- a/src/surf/fair_bottleneck.cpp +++ b/src/surf/fair_bottleneck.cpp @@ -17,7 +17,11 @@ XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(surf_maxmin); void bottleneck_solve(lmm_system_t sys) { - void *_var, *_var_next, *_cnst, *_cnst_next, *_elem; + void *_var; + void *_var_next; + void *_cnst; + void *_cnst_next; + void *_elem; lmm_variable_t var = nullptr; lmm_constraint_t cnst = nullptr; s_lmm_constraint_t s_cnst; @@ -25,7 +29,6 @@ void bottleneck_solve(lmm_system_t sys) xbt_swag_t cnst_list = nullptr; xbt_swag_t var_list = nullptr; xbt_swag_t elem_list = nullptr; - int i; static s_xbt_swag_t cnst_to_update; @@ -33,30 +36,27 @@ void bottleneck_solve(lmm_system_t sys) return; /* Init */ - xbt_swag_init(&(cnst_to_update), - xbt_swag_offset(s_cnst, saturated_constraint_set_hookup)); + xbt_swag_init(&(cnst_to_update), xbt_swag_offset(s_cnst, saturated_constraint_set_hookup)); var_list = &(sys->variable_set); XBT_DEBUG("Variable set : %d", xbt_swag_size(var_list)); xbt_swag_foreach(_var, var_list) { - var = (lmm_variable_t)_var; + var = static_cast(_var); int nb = 0; var->value = 0.0; XBT_DEBUG("Handling variable %p", var); xbt_swag_insert(var, &(sys->saturated_variable_set)); - for (i = 0; i < var->cnsts_number; i++) { + for (int i = 0; i < var->cnsts_number; i++) { if (var->cnsts[i].value == 0.0) nb++; } if ((nb == var->cnsts_number) && (var->weight > 0.0)) { - XBT_DEBUG("Err, finally, there is no need to take care of variable %p", - var); + XBT_DEBUG("Err, finally, there is no need to take care of variable %p", var); xbt_swag_remove(var, &(sys->saturated_variable_set)); var->value = 1.0; } if (var->weight <= 0.0) { - XBT_DEBUG("Err, finally, there is no need to take care of variable %p", - var); + XBT_DEBUG("Err, finally, there is no need to take care of variable %p", var); xbt_swag_remove(var, &(sys->saturated_variable_set)); } } @@ -65,12 +65,12 @@ void bottleneck_solve(lmm_system_t sys) cnst_list = &(sys->active_constraint_set); XBT_DEBUG("Active constraints : %d", xbt_swag_size(cnst_list)); xbt_swag_foreach(_cnst, cnst_list) { - cnst = (lmm_constraint_t)_cnst; + cnst = static_cast(_cnst); xbt_swag_insert(cnst, &(sys->saturated_constraint_set)); } cnst_list = &(sys->saturated_constraint_set); xbt_swag_foreach(_cnst, cnst_list) { - cnst = (lmm_constraint_t)_cnst; + cnst = static_cast(_cnst); cnst->remaining = cnst->bound; cnst->usage = 0.0; } @@ -85,19 +85,17 @@ void bottleneck_solve(lmm_system_t sys) XBT_DEBUG("Fair bottleneck done"); lmm_print(sys); } - XBT_DEBUG("******* Constraints to process: %d *******", - xbt_swag_size(cnst_list)); + XBT_DEBUG("******* Constraints to process: %d *******", xbt_swag_size(cnst_list)); xbt_swag_foreach_safe(_cnst, _cnst_next, cnst_list) { - cnst = (lmm_constraint_t)_cnst; + cnst = static_cast(_cnst); int nb = 0; XBT_DEBUG("Processing cnst %p ", cnst); elem_list = &(cnst->enabled_element_set); cnst->usage = 0.0; xbt_swag_foreach(_elem, elem_list) { - elem = (lmm_element_t)_elem; + elem = static_cast(_elem); xbt_assert(elem->variable->weight > 0); - if ((elem->value > 0) - && xbt_swag_belongs(elem->variable, var_list)) + if ((elem->value > 0) && xbt_swag_belongs(elem->variable, var_list)) nb++; } XBT_DEBUG("\tThere are %d variables", nb); @@ -110,15 +108,13 @@ void bottleneck_solve(lmm_system_t sys) continue; } cnst->usage = cnst->remaining / nb; - XBT_DEBUG("\tConstraint Usage %p : %f with %d variables", cnst, - cnst->usage, nb); + XBT_DEBUG("\tConstraint Usage %p : %f with %d variables", cnst, cnst->usage, nb); } xbt_swag_foreach_safe(_var, _var_next, var_list) { - var = (lmm_variable_t)_var; - double min_inc = - var->cnsts[0].constraint->usage / var->cnsts[0].value; - for (i = 1; i < var->cnsts_number; i++) { + var = static_cast(_var); + double min_inc = var->cnsts[0].constraint->usage / var->cnsts[0].value; + for (int i = 1; i < var->cnsts_number; i++) { lmm_element_t elm = &var->cnsts[i]; min_inc = MIN(min_inc, elm->constraint->usage / elm->value); } @@ -133,39 +129,35 @@ void bottleneck_solve(lmm_system_t sys) } xbt_swag_foreach_safe(_cnst, _cnst_next, cnst_list) { - cnst = (lmm_constraint_t)_cnst; + cnst = static_cast(_cnst); XBT_DEBUG("Updating cnst %p ", cnst); elem_list = &(cnst->enabled_element_set); xbt_swag_foreach(_elem, elem_list) { - elem = (lmm_element_t)_elem; + elem = static_cast(_elem); xbt_assert(elem->variable->weight > 0); if (cnst->sharing_policy) { - XBT_DEBUG("\tUpdate constraint %p (%g) with variable %p by %g", - cnst, cnst->remaining, elem->variable, + XBT_DEBUG("\tUpdate constraint %p (%g) with variable %p by %g", cnst, cnst->remaining, elem->variable, elem->variable->mu); - double_update(&(cnst->remaining), - elem->value * elem->variable->mu, sg_maxmin_precision); + double_update(&(cnst->remaining), elem->value * elem->variable->mu, sg_maxmin_precision); } else { - XBT_DEBUG - ("\tNon-Shared variable. Update constraint usage of %p (%g) with variable %p by %g", - cnst, cnst->usage, elem->variable, elem->variable->mu); + XBT_DEBUG("\tNon-Shared variable. Update constraint usage of %p (%g) with variable %p by %g", + cnst, cnst->usage, elem->variable, elem->variable->mu); cnst->usage = MIN(cnst->usage, elem->value * elem->variable->mu); } } if (!cnst->sharing_policy) { - XBT_DEBUG("\tUpdate constraint %p (%g) by %g", - cnst, cnst->remaining, cnst->usage); + XBT_DEBUG("\tUpdate constraint %p (%g) by %g", cnst, cnst->remaining, cnst->usage); double_update(&(cnst->remaining), cnst->usage, sg_maxmin_precision); } XBT_DEBUG("\tRemaining for %p : %g", cnst, cnst->remaining); - if (cnst->remaining == 0.0) { + if (cnst->remaining <= 0.0) { XBT_DEBUG("\tGet rid of constraint %p", cnst); xbt_swag_remove(cnst, cnst_list); xbt_swag_foreach(_elem, elem_list) { - elem = (lmm_element_t)_elem; + elem = static_cast(_elem); if (elem->variable->weight <= 0) break; if (elem->value > 0) { diff --git a/src/surf/instr_routing.cpp b/src/surf/instr_routing.cpp index bc6274b4d3..3fdbcfd05d 100644 --- a/src/surf/instr_routing.cpp +++ b/src/surf/instr_routing.cpp @@ -23,19 +23,18 @@ static std::vector currentContainer; /* push and pop, used only in static const char *instr_node_name (xbt_node_t node) { void *data = xbt_graph_node_get_data(node); - char *str = (char*)data; - return str; + return static_cast(data); } static container_t lowestCommonAncestor (container_t a1, container_t a2) { //this is only an optimization (since most of a1 and a2 share the same parent) - if (a1->father == a2->father) return a1->father; + if (a1->father == a2->father) + return a1->father; //create an array with all ancestors of a1 std::vector ancestors_a1; - container_t p; - p = a1->father; + container_t p = a1->father; while (p){ ancestors_a1.push_back(p); p = p->father; @@ -83,7 +82,8 @@ static void linkContainers (container_t src, container_t dst, xbt_dict_t filter) if (filter != nullptr){ //check if we already register this pair (we only need one direction) - char aux1[INSTR_DEFAULT_STR_SIZE], aux2[INSTR_DEFAULT_STR_SIZE]; + char aux1[INSTR_DEFAULT_STR_SIZE]; + char aux2[INSTR_DEFAULT_STR_SIZE]; snprintf (aux1, INSTR_DEFAULT_STR_SIZE, "%s%s", src->name, dst->name); snprintf (aux2, INSTR_DEFAULT_STR_SIZE, "%s%s", dst->name, src->name); if (xbt_dict_get_or_null (filter, aux1)){ @@ -118,7 +118,9 @@ static void linkContainers (container_t src, container_t dst, xbt_dict_t filter) static long long counter = 0; char key[INSTR_DEFAULT_STR_SIZE]; - snprintf (key, INSTR_DEFAULT_STR_SIZE, "%lld", counter++); + snprintf (key, INSTR_DEFAULT_STR_SIZE, "%lld", counter); + counter++; + new_pajeStartLink(SIMIX_get_clock(), father, link_type, src, "topology", key); new_pajeEndLink(SIMIX_get_clock(), father, link_type, dst, "topology", key); @@ -138,30 +140,28 @@ static void recursiveGraphExtraction(simgrid::s4u::NetZone* netzone, container_t char *child_name; //bottom-up recursion xbt_dict_foreach (netzone->children(), cursor, child_name, nz_son) { - container_t child_container = (container_t)xbt_dict_get(container->children, nz_son->name()); + container_t child_container = static_cast(xbt_dict_get(container->children, nz_son->name())); recursiveGraphExtraction(nz_son, child_container, filter); } } - { - xbt_graph_t graph = xbt_graph_new_graph (0, nullptr); - xbt_dict_t nodes = xbt_dict_new_homogeneous(nullptr); - xbt_dict_t edges = xbt_dict_new_homogeneous(nullptr); - xbt_edge_t edge = nullptr; + xbt_graph_t graph = xbt_graph_new_graph (0, nullptr); + xbt_dict_t nodes = xbt_dict_new_homogeneous(nullptr); + xbt_dict_t edges = xbt_dict_new_homogeneous(nullptr); + xbt_edge_t edge = nullptr; - xbt_dict_cursor_t cursor = nullptr; - char *edge_name; + xbt_dict_cursor_t cursor = nullptr; + char *edge_name; - static_cast(netzone)->getGraph(graph, nodes, edges); - xbt_dict_foreach(edges,cursor,edge_name,edge) { - linkContainers( - PJ_container_get((const char*) edge->src->data), - PJ_container_get((const char*) edge->dst->data), filter); - } - xbt_dict_free (&nodes); - xbt_dict_free (&edges); - xbt_graph_free_graph(graph, xbt_free_f, xbt_free_f, nullptr); + static_cast(netzone)->getGraph(graph, nodes, edges); + xbt_dict_foreach(edges,cursor,edge_name,edge) { + linkContainers( + PJ_container_get(static_cast(edge->src->data)), + PJ_container_get(static_cast(edge->dst->data)), filter); } + xbt_dict_free (&nodes); + xbt_dict_free (&edges); + xbt_graph_free_graph(graph, xbt_free_f, xbt_free_f, nullptr); } /* @@ -181,7 +181,8 @@ void sg_instr_AS_begin(sg_platf_AS_cbarg_t AS) type_t mpi = PJ_type_get_or_null ("MPI", root->type); if (mpi == nullptr){ mpi = PJ_type_container_new("MPI", root->type); - if (!TRACE_smpi_is_grouped()) PJ_type_state_new ("MPI_STATE", mpi); + if (!TRACE_smpi_is_grouped()) + PJ_type_state_new ("MPI_STATE", mpi); PJ_type_link_new ("MPI_LINK", PJ_type_get_root(), mpi, mpi); } } @@ -320,10 +321,10 @@ static void instr_routing_parse_end_platform () void instr_routing_define_callbacks () { - if (!TRACE_is_enabled()) return; //always need the call backs to ASes (we need only the root AS), //to create the rootContainer and the rootType properly - if (!TRACE_needs_platform()) return; + if (!TRACE_is_enabled() || !TRACE_needs_platform()) + return; simgrid::s4u::Link::onCreation.connect(instr_routing_parse_start_link); simgrid::s4u::onPlatformCreated.connect(instr_routing_parse_end_platform); } @@ -431,7 +432,7 @@ static void recursiveXBTGraphExtraction(xbt_graph_t graph, xbt_dict_t nodes, xbt char *child_name; //bottom-up recursion xbt_dict_foreach (netzone->children(), cursor, child_name, netzone_child) { - container_t child_container = (container_t)xbt_dict_get(container->children, netzone_child->name()); + container_t child_container = static_cast(xbt_dict_get(container->children, netzone_child->name())); recursiveXBTGraphExtraction(graph, nodes, edges, netzone_child, child_container); } } @@ -455,9 +456,8 @@ void instr_routing_platform_graph_export_graphviz (xbt_graph_t g, const char *fi unsigned int cursor = 0; xbt_node_t node = nullptr; xbt_edge_t edge = nullptr; - FILE *file = nullptr; - file = fopen(filename, "w"); + FILE *file = fopen(filename, "w"); xbt_assert(file, "Failed to open %s \n", filename); if (g->directed) @@ -468,8 +468,7 @@ void instr_routing_platform_graph_export_graphviz (xbt_graph_t g, const char *fi fprintf(file, " graph [overlap=scale]\n"); fprintf(file, " node [shape=box, style=filled]\n"); - fprintf(file, - " node [width=.3, height=.3, style=filled, color=skyblue]\n\n"); + fprintf(file, " node [width=.3, height=.3, style=filled, color=skyblue]\n\n"); xbt_dynar_foreach(g->nodes, cursor, node) { fprintf(file, " \"%s\";\n", instr_node_name(node)); @@ -484,5 +483,4 @@ void instr_routing_platform_graph_export_graphviz (xbt_graph_t g, const char *fi } fprintf(file, "}\n"); fclose(file); - } diff --git a/src/surf/network_cm02.cpp b/src/surf/network_cm02.cpp index ace3af873e..1282315a52 100644 --- a/src/surf/network_cm02.cpp +++ b/src/surf/network_cm02.cpp @@ -239,8 +239,7 @@ void NetworkCm02Model::updateActionsStateFull(double now, double delta) action->latency_ = 0.0; } if (action->latency_ == 0.0 && !(action->isSuspended())) - lmm_update_variable_weight(maxminSystem_, action->getVariable(), - action->weight_); + lmm_update_variable_weight(maxminSystem_, action->getVariable(), action->weight_); } if (TRACE_is_enabled()) { int n = lmm_get_number_of_cnst_from_var(maxminSystem_, action->getVariable()); @@ -266,13 +265,8 @@ void NetworkCm02Model::updateActionsStateFull(double now, double delta) if (action->getMaxDuration() != NO_MAX_DURATION) action->updateMaxDuration(delta); - if ((action->getRemains() <= 0) && - (lmm_get_variable_weight(action->getVariable()) > 0)) { - action->finish(); - action->setState(Action::State::done); - action->gapRemove(); - } else if (((action->getMaxDuration() != NO_MAX_DURATION) - && (action->getMaxDuration() <= 0))) { + if (((action->getRemains() <= 0) && (lmm_get_variable_weight(action->getVariable()) > 0)) || + ((action->getMaxDuration() > NO_MAX_DURATION) && (action->getMaxDuration() <= 0))) { action->finish(); action->setState(Action::State::done); action->gapRemove(); @@ -283,11 +277,8 @@ void NetworkCm02Model::updateActionsStateFull(double now, double delta) Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double size, double rate) { int failed = 0; - double bandwidth_bound; double latency = 0.0; std::vector* back_route = nullptr; - int constraints_per_variable = 0; - std::vector* route = new std::vector(); XBT_IN("(%s,%s,%g,%g)", src->cname(), dst->cname(), size, rate); @@ -310,15 +301,15 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz } NetworkCm02Action *action = new NetworkCm02Action(this, size, failed); - action->weight_ = action->latency_ = latency; - + action->weight_ = latency; + action->latency_ = latency; action->rate_ = rate; if (updateMechanism_ == UM_LAZY) { action->indexHeap_ = -1; action->lastUpdate_ = surf_get_clock(); } - bandwidth_bound = -1.0; + double bandwidth_bound = -1.0; if (sg_weight_S_parameter > 0) for (auto link : *route) action->weight_ += sg_weight_S_parameter / link->bandwidth(); @@ -340,7 +331,7 @@ Action* NetworkCm02Model::communicate(s4u::Host* src, s4u::Host* dst, double siz action->latency_); } - constraints_per_variable = route->size(); + int constraints_per_variable = route->size(); if (back_route != nullptr) constraints_per_variable += back_route->size(); @@ -456,10 +447,11 @@ void NetworkCm02Link::setBandwidth(double value) double delta = sg_weight_S_parameter / value - sg_weight_S_parameter / (bandwidth_.peak * bandwidth_.scale); lmm_variable_t var; - lmm_element_t elem = nullptr, nextelem = nullptr; + lmm_element_t elem = nullptr; + lmm_element_t nextelem = nullptr; int numelem = 0; while ((var = lmm_get_var_from_cnst_safe(model()->getMaxminSystem(), constraint(), &elem, &nextelem, &numelem))) { - NetworkCm02Action *action = (NetworkCm02Action*) lmm_variable_id(var); + NetworkCm02Action *action = static_cast(lmm_variable_id(var)); action->weight_ += delta; if (!action->isSuspended()) lmm_update_variable_weight(model()->getMaxminSystem(), action->getVariable(), action->weight_); @@ -478,7 +470,7 @@ void NetworkCm02Link::setLatency(double value) latency_.peak = value; while ((var = lmm_get_var_from_cnst_safe(model()->getMaxminSystem(), constraint(), &elem, &nextelem, &numelem))) { - NetworkCm02Action *action = (NetworkCm02Action*) lmm_variable_id(var); + NetworkCm02Action *action = static_cast(lmm_variable_id(var)); action->latCurrent_ += delta; action->weight_ += delta; if (action->rate_ < 0) @@ -509,12 +501,10 @@ NetworkCm02Action::~NetworkCm02Action() {} void NetworkCm02Action::updateRemainingLazy(double now) { - double delta = 0.0; - if (suspended_ != 0) return; - delta = now - lastUpdate_; + double delta = now - lastUpdate_; if (remains_ > 0) { XBT_DEBUG("Updating action(%p): remains was %f, last_update was: %f", this, remains_, lastUpdate_); @@ -526,14 +516,8 @@ void NetworkCm02Action::updateRemainingLazy(double now) if (maxDuration_ != NO_MAX_DURATION) double_update(&maxDuration_, delta, sg_surf_precision); - if (remains_ <= 0 && - (lmm_get_variable_weight(getVariable()) > 0)) { - finish(); - setState(Action::State::done); - - heapRemove(getModel()->getActionHeap()); - } else if (((maxDuration_ != NO_MAX_DURATION) - && (maxDuration_ <= 0))) { + if ((remains_ <= 0 && (lmm_get_variable_weight(getVariable()) > 0)) || + (((maxDuration_ > NO_MAX_DURATION) && (maxDuration_ <= 0)))){ finish(); setState(Action::State::done); heapRemove(getModel()->getActionHeap());