From 6ea2f218951f6c2abef7e604c9ed8474c989df6e Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 13 Jun 2016 11:54:31 +0200 Subject: [PATCH] dePERLify a bit the routing - Use the object, not its name (src & dst in s_sg_platf_route_cbarg_t) - this allows to move the test of their existance earlier in the process --- src/bindings/lua/lua_platf.cpp | 20 ++++++++++++------ src/s4u/s4u_as.cpp | 25 ++++++++++++---------- src/surf/AsDijkstra.cpp | 8 +++---- src/surf/AsFloyd.cpp | 37 +++++++++++++++------------------ src/surf/AsFull.cpp | 8 +++---- src/surf/AsRoutedGraph.cpp | 8 +++---- src/surf/xml/platf_private.hpp | 4 ++-- src/surf/xml/surfxml_sax_cb.cpp | 30 +++++++++++--------------- 8 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/bindings/lua/lua_platf.cpp b/src/bindings/lua/lua_platf.cpp index 9ea766aeac..8e8bffabf9 100644 --- a/src/bindings/lua/lua_platf.cpp +++ b/src/bindings/lua/lua_platf.cpp @@ -325,13 +325,17 @@ int console_add_route(lua_State *L) { lua_pushstring(L,"src"); type = lua_gettable(L,-2); lua_ensure(type == LUA_TSTRING, "Attribute 'src' must be specified for any route and must be a string."); - route.src = lua_tostring(L, -1); + const char *srcName = lua_tostring(L, -1); + route.src = sg_netcard_by_name_or_null(srcName); + lua_ensure(route.src != nullptr, "Attribute 'src=%s' of route does not name a node.", srcName); lua_pop(L,1); lua_pushstring(L,"dest"); type = lua_gettable(L,-2); lua_ensure(type == LUA_TSTRING, "Attribute 'dest' must be specified for any route and must be a string."); - route.dst = lua_tostring(L, -1); + const char *dstName = lua_tostring(L, -1); + route.dst = sg_netcard_by_name_or_null(dstName); + lua_ensure(route.dst != nullptr, "Attribute 'dst=%s' of route does not name a node.", dstName); lua_pop(L,1); lua_pushstring(L,"links"); @@ -390,26 +394,30 @@ int console_add_ASroute(lua_State *L) { lua_pushstring(L, "src"); lua_gettable(L, -2); - ASroute.src = lua_tostring(L, -1); + const char *srcName = lua_tostring(L, -1); + ASroute.src = sg_netcard_by_name_or_null(srcName); + lua_ensure(ASroute.src != nullptr, "Attribute 'src=%s' of AS route does not name a node.", srcName); lua_pop(L, 1); lua_pushstring(L, "dst"); lua_gettable(L, -2); - ASroute.dst = lua_tostring(L, -1); + const char *dstName = lua_tostring(L, -1); + ASroute.dst = sg_netcard_by_name_or_null(dstName); + lua_ensure(ASroute.dst != nullptr, "Attribute 'dst=%s' of AS route does not name a node.", dstName); lua_pop(L, 1); lua_pushstring(L, "gw_src"); lua_gettable(L, -2); const char *name = lua_tostring(L, -1); ASroute.gw_src = sg_netcard_by_name_or_null(name); - lua_ensure(ASroute.gw_src, "Attribute 'gw_src' of AS route does not name a valid machine: %s", name); + lua_ensure(ASroute.gw_src, "Attribute 'gw_src=%s' of AS route does not name a valid node", name); lua_pop(L, 1); lua_pushstring(L, "gw_dst"); lua_gettable(L, -2); name = lua_tostring(L, -1); ASroute.gw_dst = sg_netcard_by_name_or_null(name); - lua_ensure(ASroute.gw_dst, "Attribute 'gw_dst' of AS route does not name a valid machine: %s", name); + lua_ensure(ASroute.gw_dst, "Attribute 'gw_dst=%s' of AS route does not name a valid node", name); lua_pop(L, 1); lua_pushstring(L,"links"); diff --git a/src/s4u/s4u_as.cpp b/src/s4u/s4u_as.cpp index 70338f6dee..7dae44aa97 100644 --- a/src/s4u/s4u_as.cpp +++ b/src/s4u/s4u_as.cpp @@ -75,21 +75,24 @@ namespace simgrid { } void As::addBypassRoute(sg_platf_route_cbarg_t e_route){ - const char *src = e_route->src; - const char *dst = e_route->dst; - /* Argument validity checks */ if (e_route->gw_dst) { XBT_DEBUG("Load bypassASroute from %s@%s to %s@%s", - src, e_route->gw_src->name(), dst, e_route->gw_dst->name()); + e_route->src->name(), e_route->gw_src->name(), + e_route->dst->name(), e_route->gw_dst->name()); xbt_assert(!e_route->link_list->empty(), "Bypass route between %s@%s and %s@%s cannot be empty.", - src, e_route->gw_src->name(), dst, e_route->gw_dst->name()); - xbt_assert(bypassRoutes_.find({src,dst}) == bypassRoutes_.end(), "The bypass route between %s@%s and %s@%s already exists.", - src, e_route->gw_src->name(), dst, e_route->gw_dst->name()); + e_route->src->name(), e_route->gw_src->name(), + e_route->dst->name(), e_route->gw_dst->name()); + xbt_assert(bypassRoutes_.find({e_route->src->name(),e_route->dst->name()}) == bypassRoutes_.end(), + "The bypass route between %s@%s and %s@%s already exists.", + e_route->src->name(), e_route->gw_src->name(), e_route->dst->name(), e_route->gw_dst->name()); } else { - XBT_DEBUG("Load bypassRoute from %s to %s", src, dst); - xbt_assert(!e_route->link_list->empty(), "Bypass route between %s and %s cannot be empty.", src, dst); - xbt_assert(bypassRoutes_.find({src,dst}) == bypassRoutes_.end(), "The bypass route between %s and %s already exists.", src, dst); + XBT_DEBUG("Load bypassRoute from %s to %s", e_route->src->name(), e_route->dst->name()); + xbt_assert(!e_route->link_list->empty(), "Bypass route between %s and %s cannot be empty.", + e_route->src->name(), e_route->dst->name()); + xbt_assert(bypassRoutes_.find({e_route->src->name(),e_route->dst->name()}) == bypassRoutes_.end(), + "The bypass route between %s and %s already exists.", + e_route->src->name(), e_route->dst->name()); } /* Build a copy that will be stored in the dict */ @@ -98,7 +101,7 @@ namespace simgrid { newRoute->push_back(link); /* Store it */ - bypassRoutes_.insert({{src,dst}, newRoute}); + bypassRoutes_.insert({{e_route->src->name(),e_route->dst->name()}, newRoute}); } } }; // namespace simgrid::s4u diff --git a/src/surf/AsDijkstra.cpp b/src/surf/AsDijkstra.cpp index 2b701fae96..420b699c2e 100644 --- a/src/surf/AsDijkstra.cpp +++ b/src/surf/AsDijkstra.cpp @@ -314,10 +314,10 @@ AsDijkstra::AsDijkstra(const char*name, bool cached) void AsDijkstra::addRoute(sg_platf_route_cbarg_t route) { - const char *srcName = route->src; - const char *dstName = route->dst; - NetCard *src = sg_netcard_by_name_or_null(srcName); - NetCard *dst = sg_netcard_by_name_or_null(dstName); + NetCard *src = route->src; + NetCard *dst = route->dst; + const char *srcName = src->name(); + const char *dstName = dst->name(); addRouteCheckParams(route); diff --git a/src/surf/AsFloyd.cpp b/src/surf/AsFloyd.cpp index 887d5a579e..99ab0dff86 100644 --- a/src/surf/AsFloyd.cpp +++ b/src/surf/AsFloyd.cpp @@ -87,9 +87,6 @@ void AsFloyd::addRoute(sg_platf_route_cbarg_t route) /* set the size of table routing */ int table_size = (int)xbt_dynar_length(vertices_); - NetCard *src = sg_netcard_by_name_or_null(route->src); - NetCard *dst = sg_netcard_by_name_or_null(route->dst); - addRouteCheckParams(route); if(!linkTable_) { @@ -109,27 +106,27 @@ void AsFloyd::addRoute(sg_platf_route_cbarg_t route) /* Check that the route does not already exist */ if (route->gw_dst) // AS route (to adapt the error message, if any) - xbt_assert(nullptr == TO_FLOYD_LINK(src->id(), dst->id()), + xbt_assert(nullptr == TO_FLOYD_LINK(route->src->id(), route->dst->id()), "The route between %s@%s and %s@%s already exists (Rq: routes are symmetrical by default).", - src->name(),route->gw_src->name(),dst->name(),route->gw_dst->name()); + route->src->name(),route->gw_src->name(),route->dst->name(),route->gw_dst->name()); else - xbt_assert(nullptr == TO_FLOYD_LINK(src->id(), dst->id()), - "The route between %s and %s already exists (Rq: routes are symmetrical by default).", src->name(),dst->name()); + xbt_assert(nullptr == TO_FLOYD_LINK(route->src->id(), route->dst->id()), + "The route between %s and %s already exists (Rq: routes are symmetrical by default).", route->src->name(),route->dst->name()); - TO_FLOYD_LINK(src->id(), dst->id()) = newExtendedRoute(hierarchy_, route, 1); - TO_FLOYD_PRED(src->id(), dst->id()) = src->id(); - TO_FLOYD_COST(src->id(), dst->id()) = (TO_FLOYD_LINK(src->id(), dst->id()))->link_list->size(); + TO_FLOYD_LINK(route->src->id(), route->dst->id()) = newExtendedRoute(hierarchy_, route, 1); + TO_FLOYD_PRED(route->src->id(), route->dst->id()) = route->src->id(); + TO_FLOYD_COST(route->src->id(), route->dst->id()) = (TO_FLOYD_LINK(route->src->id(), route->dst->id()))->link_list->size(); if (route->symmetrical == true) { if (route->gw_dst) // AS route (to adapt the error message, if any) - xbt_assert(nullptr == TO_FLOYD_LINK(dst->id(), src->id()), + xbt_assert(nullptr == TO_FLOYD_LINK(route->dst->id(), route->src->id()), "The route between %s@%s and %s@%s already exists. You should not declare the reverse path as symmetrical.", - dst->name(),route->gw_dst->name(),src->name(),route->gw_src->name()); + route->dst->name(),route->gw_dst->name(),route->src->name(),route->gw_src->name()); else - xbt_assert(nullptr == TO_FLOYD_LINK(dst->id(), src->id()), + xbt_assert(nullptr == TO_FLOYD_LINK(route->dst->id(), route->src->id()), "The route between %s and %s already exists. You should not declare the reverse path as symmetrical.", - dst->name(),src->name()); + route->dst->name(),route->src->name()); if(route->gw_dst && route->gw_src) { NetCard* gw_tmp = route->gw_src; @@ -138,14 +135,14 @@ void AsFloyd::addRoute(sg_platf_route_cbarg_t route) } if(!route->gw_src && !route->gw_dst) - XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst->name(), src->name()); + XBT_DEBUG("Load Route from \"%s\" to \"%s\"", route->dst->name(), route->src->name()); else - XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst->name(), - route->gw_src->name(), src->name(), route->gw_dst->name()); + XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", route->dst->name(), + route->gw_src->name(), route->src->name(), route->gw_dst->name()); - TO_FLOYD_LINK(dst->id(), src->id()) = newExtendedRoute(hierarchy_, route, 0); - TO_FLOYD_PRED(dst->id(), src->id()) = dst->id(); - TO_FLOYD_COST(dst->id(), src->id()) = (TO_FLOYD_LINK(dst->id(), src->id()))->link_list->size(); /* count of links, old model assume 1 */ + TO_FLOYD_LINK(route->dst->id(), route->src->id()) = newExtendedRoute(hierarchy_, route, 0); + TO_FLOYD_PRED(route->dst->id(), route->src->id()) = route->dst->id(); + TO_FLOYD_COST(route->dst->id(), route->src->id()) = (TO_FLOYD_LINK(route->dst->id(), route->src->id()))->link_list->size(); /* count of links, old model assume 1 */ } } diff --git a/src/surf/AsFull.cpp b/src/surf/AsFull.cpp index 2678d36b63..c2b4ca50ea 100644 --- a/src/surf/AsFull.cpp +++ b/src/surf/AsFull.cpp @@ -85,10 +85,10 @@ void AsFull::getRouteAndLatency(NetCard *src, NetCard *dst, sg_platf_route_cbarg void AsFull::addRoute(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); + NetCard *src_net_elm = route->src; + NetCard *dst_net_elm = route->dst; + const char *src = src_net_elm->name(); + const char *dst = dst_net_elm->name(); addRouteCheckParams(route); diff --git a/src/surf/AsRoutedGraph.cpp b/src/surf/AsRoutedGraph.cpp index 5ba0b40433..ee1f3c5c55 100644 --- a/src/surf/AsRoutedGraph.cpp +++ b/src/surf/AsRoutedGraph.cpp @@ -221,10 +221,10 @@ void AsRoutedGraph::getRouteCheckParams(NetCard *src, NetCard *dst) src->name(), dst->name(), src_as->name(), dst_as->name(), name()); } void AsRoutedGraph::addRouteCheckParams(sg_platf_route_cbarg_t route) { - const char *srcName = route->src; - const char *dstName = route->dst; - NetCard *src = sg_netcard_by_name_or_null(srcName); - NetCard *dst = sg_netcard_by_name_or_null(dstName); + NetCard *src = route->src; + NetCard *dst = route->dst; + const char *srcName = src->name(); + const char *dstName = dst->name(); if(!route->gw_dst && !route->gw_src) { XBT_DEBUG("Load Route from \"%s\" to \"%s\"", srcName, dstName); diff --git a/src/surf/xml/platf_private.hpp b/src/surf/xml/platf_private.hpp index 899c37bd46..67c7768f3b 100644 --- a/src/surf/xml/platf_private.hpp +++ b/src/surf/xml/platf_private.hpp @@ -87,8 +87,8 @@ typedef struct s_sg_platf_peer_cbarg { typedef struct s_sg_platf_route_cbarg *sg_platf_route_cbarg_t; typedef struct s_sg_platf_route_cbarg { bool symmetrical; - const char *src; - const char *dst; + sg_netcard_t src; + sg_netcard_t dst; sg_netcard_t gw_src; sg_netcard_t gw_dst; std::vector *link_list; diff --git a/src/surf/xml/surfxml_sax_cb.cpp b/src/surf/xml/surfxml_sax_cb.cpp index 4a75fdb0f0..a4f90145e7 100644 --- a/src/surf/xml/surfxml_sax_cb.cpp +++ b/src/surf/xml/surfxml_sax_cb.cpp @@ -724,8 +724,9 @@ void ETag_surfxml_route(void){ s_sg_platf_route_cbarg_t route; memset(&route,0,sizeof(route)); - route.src = A_surfxml_route_src; - route.dst = A_surfxml_route_dst; + + route.src = sg_netcard_by_name_or_null(A_surfxml_route_src); // tested to not be NULL in start tag + route.dst = sg_netcard_by_name_or_null(A_surfxml_route_dst); // tested to not be NULL in start tag route.gw_src = nullptr; route.gw_dst = nullptr; route.link_list = new std::vector(); @@ -747,18 +748,11 @@ void ETag_surfxml_ASroute(void){ s_sg_platf_route_cbarg_t ASroute; memset(&ASroute,0,sizeof(ASroute)); - ASroute.src = A_surfxml_ASroute_src; - ASroute.dst = A_surfxml_ASroute_dst; - - ASroute.gw_src = sg_netcard_by_name_or_null(A_surfxml_ASroute_gw___src); - ASroute.gw_dst = sg_netcard_by_name_or_null(A_surfxml_ASroute_gw___dst); + ASroute.src = sg_netcard_by_name_or_null(A_surfxml_ASroute_src); // tested to not be NULL in start tag + ASroute.dst = sg_netcard_by_name_or_null(A_surfxml_ASroute_dst); // tested to not be NULL in start tag - if (A_surfxml_ASroute_gw___src && !ASroute.gw_src) - surf_parse_error("gw_src=\"%s\" not found for ASroute from \"%s\" to \"%s\"", - A_surfxml_ASroute_gw___src, ASroute.src, ASroute.dst); - if (A_surfxml_ASroute_gw___dst && !ASroute.gw_dst) - surf_parse_error("gw_dst=\"%s\" not found for ASroute from \"%s\" to \"%s\"", - A_surfxml_ASroute_gw___dst, ASroute.src, ASroute.dst); + ASroute.gw_src = sg_netcard_by_name_or_null(A_surfxml_ASroute_gw___src); // tested to not be NULL in start tag + ASroute.gw_dst = sg_netcard_by_name_or_null(A_surfxml_ASroute_gw___dst); // tested to not be NULL in start tag ASroute.link_list = new std::vector(); @@ -788,8 +782,8 @@ void ETag_surfxml_bypassRoute(void){ s_sg_platf_route_cbarg_t route; memset(&route,0,sizeof(route)); - route.src = A_surfxml_bypassRoute_src; - route.dst = A_surfxml_bypassRoute_dst; + route.src = sg_netcard_by_name_or_null(A_surfxml_bypassRoute_src); // tested to not be NULL in start tag + route.dst = sg_netcard_by_name_or_null(A_surfxml_bypassRoute_dst); // tested to not be NULL in start tag route.gw_src = nullptr; route.gw_dst = nullptr; route.symmetrical = false; @@ -810,9 +804,9 @@ void ETag_surfxml_bypassASroute(void){ s_sg_platf_route_cbarg_t ASroute; memset(&ASroute,0,sizeof(ASroute)); - ASroute.src = A_surfxml_bypassASroute_src; - ASroute.dst = A_surfxml_bypassASroute_dst; - ASroute.link_list = new std::vector(); + ASroute.src = sg_netcard_by_name_or_null(A_surfxml_bypassASroute_src); + ASroute.dst = sg_netcard_by_name_or_null(A_surfxml_bypassASroute_dst); + ASroute.link_list = new std::vector(); unsigned int cpt; char *link_name; xbt_dynar_foreach(parsed_link_list, cpt, link_name) { -- 2.20.1