From e2b07f653b8942d8d4cb4eab2f4ae15e9156b3ac Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 14 Feb 2016 13:31:15 +0100 Subject: [PATCH] routing: factorize the argument checking of parseRoute()s --- include/simgrid/platf.h | 2 +- src/surf/surf_routing_dijkstra.cpp | 20 ++----------- src/surf/surf_routing_floyd.cpp | 15 ++-------- src/surf/surf_routing_full.cpp | 46 ++++-------------------------- src/surf/surf_routing_generic.cpp | 23 ++++++++++++++- src/surf/surf_routing_generic.hpp | 4 ++- 6 files changed, 37 insertions(+), 73 deletions(-) diff --git a/include/simgrid/platf.h b/include/simgrid/platf.h index 34d1d008a1..1bd0067a65 100644 --- a/include/simgrid/platf.h +++ b/include/simgrid/platf.h @@ -280,7 +280,7 @@ XBT_PUBLIC(void) sg_platf_new_mount(sg_platf_mount_cbarg_t mount); XBT_PUBLIC(void) sg_platf_new_process(sg_platf_process_cbarg_t process); // Add route and Asroute without xml file with those functions -XBT_PUBLIC(void) sg_platf_route_begin (sg_platf_route_cbarg_t route); // Initialize route +XBT_PUBLIC(void) sg_platf_route_begin (sg_platf_route_cbarg_t route); // Initialize route FIXME:KILLME XBT_PUBLIC(void) sg_platf_route_end (sg_platf_route_cbarg_t route); // Finalize and add a route XBT_PUBLIC(void) sg_platf_route_add_link (const char* link_id, sg_platf_route_cbarg_t route); // Add a link to link list diff --git a/src/surf/surf_routing_dijkstra.cpp b/src/surf/surf_routing_dijkstra.cpp index 88cc782428..940269bcac 100644 --- a/src/surf/surf_routing_dijkstra.cpp +++ b/src/surf/surf_routing_dijkstra.cpp @@ -380,23 +380,10 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) { const char *src = route->src; const char *dst = route->dst; - - if(!route->gw_dst && !route->gw_src) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst); - else{ - XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", src, - route->gw_src->name(), dst, route->gw_dst->name()); - if(route->gw_dst->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The gw_dst '%s' does not exist!",route->gw_dst->name()); - if(route->gw_src->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The gw_src '%s' does not exist!",route->gw_src->name()); - } - NetCard *src_net_elm = sg_netcard_by_name_or_null(src); NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); - xbt_assert(src_net_elm, "Network elements %s not found", src); - xbt_assert(dst_net_elm, "Network elements %s not found", dst); + parseRouteCheckParams(route); /* Create the topology graph */ if(!p_routeGraph) @@ -412,8 +399,7 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) if(!route->gw_dst && !route->gw_src) XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst, src); else - XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst, - route->gw_dst->name(), src, route->gw_src->name()); + XBT_DEBUG("Load ASroute from %s@%s to %s@%s", dst, route->gw_dst->name(), src, route->gw_src->name()); xbt_dynar_t nodes = xbt_graph_get_nodes(p_routeGraph); xbt_node_t node_s_v = xbt_dynar_get_as(nodes, src_net_elm->id(), xbt_node_t); @@ -421,7 +407,7 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route) xbt_edge_t edge = xbt_graph_get_edge(p_routeGraph, node_e_v, node_s_v); if (edge) - THROWF(arg_error,0, "Route from '%s' to '%s' already exists",src,dst); + THROWF(arg_error,0, "Route from %s@%s to %s@%s already exists", dst, route->gw_dst->name(), src, route->gw_src->name()); if (route->gw_dst && route->gw_src) { NetCard *gw_tmp = route->gw_src; diff --git a/src/surf/surf_routing_floyd.cpp b/src/surf/surf_routing_floyd.cpp index b1b6909c3a..07543520e8 100644 --- a/src/surf/surf_routing_floyd.cpp +++ b/src/surf/surf_routing_floyd.cpp @@ -143,8 +143,7 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route) NetCard *src_net_elm = sg_netcard_by_name_or_null(src); NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); - xbt_assert(src_net_elm, "Network elements %s not found", src); - xbt_assert(dst_net_elm, "Network elements %s not found", dst); + parseRouteCheckParams(route); if(!p_linkTable) { @@ -162,16 +161,6 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route) TO_FLOYD_LINK(i, j) = NULL; /* fixed, missing in the previous version */ } } - if(!route->gw_dst && !route->gw_src) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst); - else{ - XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", src, - route->gw_src->name(), dst, route->gw_dst->name()); - if(route->gw_dst->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The dst_gateway '%s' does not exist!",route->gw_dst->name()); - if(route->gw_src->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The src_gateway '%s' does not exist!",route->gw_src->name()); - } if(TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id())) { @@ -263,7 +252,7 @@ void AsFloyd::Seal(){ for (j = 0; j < table_size; j++) { TO_FLOYD_COST(i, j) = DBL_MAX; TO_FLOYD_PRED(i, j) = -1; - TO_FLOYD_LINK(i, j) = NULL; /* fixed, missing in the previous version */ + TO_FLOYD_LINK(i, j) = NULL; } } diff --git a/src/surf/surf_routing_full.cpp b/src/surf/surf_routing_full.cpp index 1ec430882b..5bd17ec881 100644 --- a/src/surf/surf_routing_full.cpp +++ b/src/surf/surf_routing_full.cpp @@ -137,15 +137,10 @@ void AsFull::parseRoute(sg_platf_route_cbarg_t route) NetCard *src_net_elm = sg_netcard_by_name_or_null(src); NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); - xbt_assert(src_net_elm, "Network elements %s not found", src); - xbt_assert(dst_net_elm, "Network elements %s not found", dst); + parseRouteCheckParams(route); size_t table_size = xbt_dynar_length(vertices_); - xbt_assert(!xbt_dynar_is_empty(route->link_list), - "Invalid count of links, must be greater than zero (%s,%s)", - src, dst); - if (!p_routingTable) p_routingTable = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size); @@ -174,39 +169,11 @@ void AsFull::parseRoute(sg_platf_route_cbarg_t route) if (!route->gw_src && !route->gw_dst) XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst); else { - // FIXME We can call a gw which is down the current AS (cf g5k.xml) but not upper. - // AS_t subas = xbt_dict_get_or_null(rc->routing_sons, src); - // if (subas == NULL) - // surf_parse_error("The source of an ASroute must be a sub-AS " - // "declared within the current AS, " - // "but '%s' is not an AS within '%s'", src, rc->name); - // if (subas->to_index - // && xbt_dict_get_or_null(subas->to_index, route->src_gateway) == NULL) - // surf_parse_error("In an ASroute, source gateway must be part of " - // "the source sub-AS (in particular, being in a " - // "sub-sub-AS is not allowed), " - // "but '%s' is not in '%s'.", - // route->src_gateway, subas->name); - // - // subas = xbt_dict_get_or_null(rc->routing_sons, dst); - // if (subas == NULL) - // surf_parse_error("The destination of an ASroute must be a sub-AS " - // "declared within the current AS, " - // "but '%s' is not an AS within '%s'", dst, rc->name); - // if (subas->to_index - // && xbt_dict_get_or_null(subas->to_index, route->dst_gateway) == NULL) - // surf_parse_error("In an ASroute, destination gateway must be " - // "part of the destination sub-AS (in particular, " - // "in a sub-sub-AS is not allowed), " - // "but '%s' is not in '%s'.", - // route->dst_gateway, subas->name); XBT_DEBUG("Load ASroute from \"%s\" to \"%s\"", src, dst); - if (!route->gw_src || - route->gw_src->getRcType() == SURF_NETWORK_ELEMENT_NULL) - surf_parse_error("The src_gateway \"%s\" does not exist!", - route->gw_src ? route->gw_src->name() : "(null)"); - if (!route->gw_dst || - route->gw_dst->getRcType() == SURF_NETWORK_ELEMENT_NULL) + if (!route->gw_src || route->gw_src->getRcType() == SURF_NETWORK_ELEMENT_NULL) + surf_parse_error("The src_gateway \"%s\" does not exist!", + route->gw_src ? route->gw_src->name() : "(null)"); + if (!route->gw_dst || route->gw_dst->getRcType() == SURF_NETWORK_ELEMENT_NULL) surf_parse_error("The dst_gateway \"%s\" does not exist!", route->gw_dst ? route->gw_dst->name() : "(null)"); XBT_DEBUG("ASroute goes from \"%s\" to \"%s\"", @@ -225,8 +192,7 @@ void AsFull::parseRoute(sg_platf_route_cbarg_t route) if (TO_ROUTE_FULL(dst_net_elm->id(), src_net_elm->id())) { char *link_name; unsigned int i; - xbt_dynar_t link_route_to_test = - xbt_dynar_new(sizeof(sg_routing_link_t), NULL); + xbt_dynar_t link_route_to_test = xbt_dynar_new(sizeof(sg_routing_link_t), NULL); for (i = xbt_dynar_length(route->link_list); i > 0; i--) { link_name = xbt_dynar_get_as(route->link_list, i - 1, char *); void *link = Link::byName(link_name); diff --git a/src/surf/surf_routing_generic.cpp b/src/surf/surf_routing_generic.cpp index 4ecc89f55c..11c4e7297f 100644 --- a/src/surf/surf_routing_generic.cpp +++ b/src/surf/surf_routing_generic.cpp @@ -351,7 +351,7 @@ sg_platf_route_cbarg_t AsGeneric::newExtendedRoute(e_surf_routing_hierarchy_t hi if (hierarchy == SURF_ROUTING_RECURSIVE) { xbt_assert(routearg->gw_src && routearg->gw_dst, - "NULL is obviously a bad gateway"); + "NULL is obviously a deficient gateway"); /* remember not erase the gateway names */ result->gw_src = routearg->gw_src; @@ -388,6 +388,27 @@ void AsGeneric::srcDstCheck(NetCard *src, NetCard *dst) "Internal error: route destination %s@%s is not in AS %s as expected (route source: %s@%s). Please report that bug.", src->name(), dst->name(), src_as->name_, dst_as->name_, name_); } +void AsGeneric::parseRouteCheckParams(sg_platf_route_cbarg_t route) { + const char *src = route->src; + const char *dst = route->dst; + NetCard *src_net_elm = sg_netcard_by_name_or_null(src); + NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst); + + if(!route->gw_dst && !route->gw_src) { + XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst); + xbt_assert(src_net_elm, "Cannot add a route from %s to %s: %s does not exist.", src, dst, src); + xbt_assert(dst_net_elm, "Cannot add a route from %s to %s: %s does not exist.", src, dst, dst); + xbt_assert(!xbt_dynar_is_empty(route->link_list), "Empty route (between %s and %s) forbidden.", src, dst); + } else { + XBT_DEBUG("Load ASroute from %s@%s to %s@%s", src, route->gw_src->name(), dst, route->gw_dst->name()); + xbt_assert(src_net_elm, "Cannot add a route from %s@%s to %s@%s: %s does not exist.", + src,route->gw_src->name(), dst,route->gw_dst->name(), src); + xbt_assert(dst_net_elm, "Cannot add a route from %s@%s to %s@%s: %s does not exist.", + src,route->gw_src->name(), dst,route->gw_dst->name(), dst); + xbt_assert(!xbt_dynar_is_empty(route->link_list), "Empty route (between %s@%s and %s@%s) forbidden.", + src,route->gw_src->name(), dst,route->gw_dst->name()); + } +} } } diff --git a/src/surf/surf_routing_generic.hpp b/src/surf/surf_routing_generic.hpp index 33efa904b1..8418f673a1 100644 --- a/src/surf/surf_routing_generic.hpp +++ b/src/surf/surf_routing_generic.hpp @@ -28,7 +28,9 @@ public: virtual void parseBypassroute(sg_platf_route_cbarg_t e_route) override; virtual sg_platf_route_cbarg_t newExtendedRoute(e_surf_routing_hierarchy_t hierarchy, sg_platf_route_cbarg_t routearg, int change_order); - virtual void srcDstCheck(NetCard *src, NetCard *dst); +protected: + void srcDstCheck(NetCard *src, NetCard *dst); + void parseRouteCheckParams(sg_platf_route_cbarg_t route); }; } -- 2.20.1