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

diff --git a/src/bindings/lua/lua_platf.cpp b/src/bindings/lua/lua_platf.cpp
index 9ea766a..8e8bffa 100644 (file)
--- 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 70338f6..7dae44a 100644 (file)
--- 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 2b701fa..420b699 100644 (file)
--- 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 887d5a5..99ab0df 100644 (file)
--- 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 2678d36..c2b4ca5 100644 (file)
--- 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 5ba0b40..ee1f3c5 100644 (file)
--- 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 899c37b..67c7768 100644 (file)
--- 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*> *link_list;

diff --git a/src/surf/xml/surfxml_sax_cb.cpp b/src/surf/xml/surfxml_sax_cb.cpp
index 4a75fdb..a4f9014 100644 (file)
--- 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<Link*>();
@@ -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<Link*>();
 
@@ -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<Link*>();
+  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<Link*>();
   unsigned int cpt;
   char *link_name;
   xbt_dynar_foreach(parsed_link_list, cpt, link_name) {