further cleanups in routing: check if route already exists uniformly

diff --git a/src/surf/surf_routing_dijkstra.cpp b/src/surf/surf_routing_dijkstra.cpp
index b6fae6e..1e9a319 100644 (file)
--- a/src/surf/surf_routing_dijkstra.cpp
+++ b/src/surf/surf_routing_dijkstra.cpp
@@ -378,10 +378,10 @@ AsDijkstra::AsDijkstra(const char*name, bool cached)
 
 void AsDijkstra::parseRoute(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);
+  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);
 
   parseRouteCheckParams(route);
 
@@ -391,23 +391,26 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route)
   if(!graphNodeMap_)
     graphNodeMap_ = xbt_dict_new_homogeneous(&graph_node_map_elem_free);
 
+  /* we don't check whether the route already exist, because the algorithm may find another path through some other nodes */
+
+  /* Add the route to the base */
   sg_platf_route_cbarg_t e_route = newExtendedRoute(hierarchy_, route, 1);
-  newRoute(src_net_elm->id(), dst_net_elm->id(), e_route);
+  newRoute(src->id(), dst->id(), e_route);
 
   // Symmetrical YES
   if (route->symmetrical == TRUE) {
     if(!route->gw_dst && !route->gw_src)
-      XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst, src);
+      XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dstName, srcName);
     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", dstName, route->gw_dst->name(), srcName, route->gw_src->name());
 
     xbt_dynar_t nodes = xbt_graph_get_nodes(routeGraph_);
-    xbt_node_t node_s_v = xbt_dynar_get_as(nodes, src_net_elm->id(), xbt_node_t);
-    xbt_node_t node_e_v = xbt_dynar_get_as(nodes, dst_net_elm->id(), xbt_node_t);
+    xbt_node_t node_s_v = xbt_dynar_get_as(nodes, src->id(), xbt_node_t);
+    xbt_node_t node_e_v = xbt_dynar_get_as(nodes, dst->id(), xbt_node_t);
     xbt_edge_t edge = xbt_graph_get_edge(routeGraph_, node_e_v, node_s_v);
 
     if (edge)
-      THROWF(arg_error,0, "Route from %s@%s to %s@%s already exists", dst, route->gw_dst->name(), src, route->gw_src->name());
+      THROWF(arg_error,0, "Route from %s@%s to %s@%s already exists", dstName, route->gw_dst->name(), srcName, route->gw_src->name());
 
     if (route->gw_dst && route->gw_src) {
       NetCard *gw_tmp = route->gw_src;
@@ -415,7 +418,7 @@ void AsDijkstra::parseRoute(sg_platf_route_cbarg_t route)
       route->gw_dst = gw_tmp;
     }
     sg_platf_route_cbarg_t link_route_back = newExtendedRoute(hierarchy_, route, 0);
-    newRoute(dst_net_elm->id(), src_net_elm->id(), link_route_back);
+    newRoute(dst->id(), src->id(), link_route_back);
   }
   xbt_dynar_free(&route->link_list);
 }

diff --git a/src/surf/surf_routing_floyd.cpp b/src/surf/surf_routing_floyd.cpp
index f89b229..487460c 100644 (file)
--- a/src/surf/surf_routing_floyd.cpp
+++ b/src/surf/surf_routing_floyd.cpp
@@ -131,68 +131,50 @@ static int floyd_pointer_resource_cmp(const void *a, const void *b) {
 
 void AsFloyd::parseRoute(sg_platf_route_cbarg_t route)
 {
-  const char *src = route->src;
-  const char *dst = route->dst;
-
   /* set the size of table routing */
   int table_size = (int)xbt_dynar_length(vertices_);
 
-  NetCard *src_net_elm = sg_netcard_by_name_or_null(src);
-  NetCard *dst_net_elm = sg_netcard_by_name_or_null(dst);
+  NetCard *src = sg_netcard_by_name_or_null(route->src);
+  NetCard *dst = sg_netcard_by_name_or_null(route->dst);
 
   parseRouteCheckParams(route);
 
-  if(!linkTable_)
-  {
-    int i,j;
+  if(!linkTable_) {
     /* Create Cost, Predecessor and Link tables */
     costTable_ = xbt_new0(double, table_size * table_size);       /* link cost from host to host */
     predecessorTable_ = xbt_new0(int, table_size * table_size);  /* predecessor host numbers */
     linkTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size);    /* actual link between src and dst */
 
     /* Initialize costs and predecessors */
-    for (i = 0; i < table_size; i++)
-      for (j = 0; j < table_size; j++) {
+    for (int i = 0; i < table_size; i++)
+      for (int 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;
       }
   }
 
-  if(TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id()))
-  {
-
-    char * link_name;
-    unsigned int cpt;
-    xbt_dynar_t link_route_to_test = xbt_dynar_new(sizeof(Link*), NULL);
-    xbt_dynar_foreach(route->link_list,cpt,link_name)
-    {
-      void *link = Link::byName(link_name);
-      xbt_assert(link,"Link : '%s' doesn't exists.",link_name);
-      xbt_dynar_push(link_route_to_test,&link);
-    }
-    xbt_assert(!xbt_dynar_compare(
-        TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id())->link_list,
-        link_route_to_test,
-        (int_f_cpvoid_cpvoid_t) floyd_pointer_resource_cmp),
-        "The route between \"%s\" and \"%s\" already exists", src,dst);
-  }
+  /* 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()),
+        "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());
   else
-  {
-    TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id()) =
-        newExtendedRoute(hierarchy_, route, 1);
-    TO_FLOYD_PRED(src_net_elm->id(), dst_net_elm->id()) = src_net_elm->id();
-    TO_FLOYD_COST(src_net_elm->id(), dst_net_elm->id()) =
-        ((TO_FLOYD_LINK(src_net_elm->id(), dst_net_elm->id()))->link_list)->used;   /* count of links, old model assume 1 */
-  }
+    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());
+
+  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)->used;
+
 
   if (route->symmetrical == TRUE) {
-    if(TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id()))
+    if(TO_FLOYD_LINK(dst->id(), src->id()))
     {
       if(!route->gw_dst && !route->gw_src)
-        XBT_DEBUG("See Route from \"%s\" to \"%s\"", dst, src);
+        XBT_DEBUG("See Route from \"%s\" to \"%s\"", dst->name(), src->name());
       else
-        XBT_DEBUG("See ASroute from \"%s(%s)\" to \"%s(%s)\"", dst, route->gw_src->name(), src, route->gw_dst->name());
+        XBT_DEBUG("See ASroute from \"%s(%s)\" to \"%s(%s)\"", dst->name(), route->gw_src->name(), src->name(), route->gw_dst->name());
 
       char * link_name;
       xbt_dynar_t link_route_to_test = xbt_dynar_new(sizeof(Link*), NULL);
@@ -203,10 +185,10 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route)
         xbt_dynar_push(link_route_to_test,&link);
       }
       xbt_assert(!xbt_dynar_compare(
-          TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id())->link_list,
+          TO_FLOYD_LINK(dst->id(), src->id())->link_list,
           link_route_to_test,
           (int_f_cpvoid_cpvoid_t) floyd_pointer_resource_cmp),
-          "The route between \"%s\" and \"%s\" already exists", src,dst);
+          "The route between \"%s\" and \"%s\" already exists", src->name(),dst->name());
     }
     else {
 
@@ -217,23 +199,22 @@ void AsFloyd::parseRoute(sg_platf_route_cbarg_t route)
       }
 
       if(!route->gw_src && !route->gw_dst)
-        XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst, src);
+        XBT_DEBUG("Load Route from \"%s\" to \"%s\"", dst->name(), src->name());
       else
-        XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst,
-            route->gw_src->name(), src, route->gw_dst->name());
+        XBT_DEBUG("Load ASroute from \"%s(%s)\" to \"%s(%s)\"", dst->name(),
+            route->gw_src->name(), src->name(), route->gw_dst->name());
 
-      TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id()) =
+      TO_FLOYD_LINK(dst->id(), src->id()) =
          newExtendedRoute(hierarchy_, route, 0);
-      TO_FLOYD_PRED(dst_net_elm->id(), src_net_elm->id()) = dst_net_elm->id();
-      TO_FLOYD_COST(dst_net_elm->id(), src_net_elm->id()) =
-          ((TO_FLOYD_LINK(dst_net_elm->id(), src_net_elm->id()))->link_list)->used;   /* count of links, old model assume 1 */
+      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)->used;   /* count of links, old model assume 1 */
     }
   }
   xbt_dynar_free(&route->link_list);
 }
 
 void AsFloyd::Seal(){
-  unsigned int i, j, a, b, c;
 
   /* set the size of table routing */
   size_t table_size = xbt_dynar_length(vertices_);
@@ -245,8 +226,8 @@ void AsFloyd::Seal(){
     linkTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size);    /* actual link between src and dst */
 
     /* Initialize costs and predecessors */
-    for (i = 0; i < table_size; i++)
-      for (j = 0; j < table_size; j++) {
+    for (unsigned int i = 0; i < table_size; i++)
+      for (unsigned int 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;
@@ -255,7 +236,7 @@ void AsFloyd::Seal(){
 
   /* Add the loopback if needed */
   if (routing_platf->loopback_ && hierarchy_ == SURF_ROUTING_BASE) {
-    for (i = 0; i < table_size; i++) {
+    for (unsigned int i = 0; i < table_size; i++) {
       sg_platf_route_cbarg_t e_route = TO_FLOYD_LINK(i, i);
       if (!e_route) {
         e_route = xbt_new0(s_sg_platf_route_cbarg_t, 1);
@@ -270,9 +251,9 @@ void AsFloyd::Seal(){
     }
   }
   /* Calculate path costs */
-  for (c = 0; c < table_size; c++) {
-    for (a = 0; a < table_size; a++) {
-      for (b = 0; b < table_size; b++) {
+  for (unsigned int c = 0; c < table_size; c++) {
+    for (unsigned int a = 0; a < table_size; a++) {
+      for (unsigned int b = 0; b < table_size; b++) {
         if (TO_FLOYD_COST(a, c) < DBL_MAX && TO_FLOYD_COST(c, b) < DBL_MAX) {
           if (TO_FLOYD_COST(a, b) == DBL_MAX ||
               (TO_FLOYD_COST(a, c) + TO_FLOYD_COST(c, b) <

diff --git a/src/surf/surf_routing_full.cpp b/src/surf/surf_routing_full.cpp
index 9cf68f1..427af14 100644 (file)
--- a/src/surf/surf_routing_full.cpp
+++ b/src/surf/surf_routing_full.cpp
@@ -144,44 +144,18 @@ void AsFull::parseRoute(sg_platf_route_cbarg_t route)
   if (!routingTable_)
     routingTable_ = xbt_new0(sg_platf_route_cbarg_t, table_size * table_size);
 
-  if (TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())) {
-    char *link_name;
-    unsigned int i;
-    xbt_dynar_t link_route_to_test =
-        xbt_dynar_new(sizeof(Link*), NULL);
-    xbt_dynar_foreach(route->link_list, i, link_name) {
-      void *link = Link::byName(link_name);
-      xbt_assert(link, "Link : '%s' doesn't exists.", link_name);
-      xbt_dynar_push(link_route_to_test, &link);
-    }
-    if (xbt_dynar_compare(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list,
-        link_route_to_test, full_pointer_resource_cmp)) {
-      surf_parse_error("A route between \"%s\" and \"%s\" already exists "
-          "with a different content. "
-          "If you are trying to define a reverse route, "
-          "you must set the symmetrical=no attribute to "
-          "your routes tags.", src, dst);
-    } else {
-      surf_parse_warn("Ignoring the identical redefinition of the route "
-          "between \"%s\" and \"%s\"", src, dst);
-    }
-  } else {
-    if (!route->gw_src && !route->gw_dst)
-      XBT_DEBUG("Load Route from \"%s\" to \"%s\"", src, dst);
-    else {
-      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)
-      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\"",
-                route->gw_src->name(), route->gw_dst->name());
-    }
-    TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()) = newExtendedRoute(hierarchy_, route, 1);
-    xbt_dynar_shrink(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list, 0);
-  }
+  /* 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_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()),
+        "The route between %s@%s and %s@%s already exists (Rq: routes are symmetrical by default).",
+        src,route->gw_src->name(),dst,route->gw_dst->name());
+  else
+    xbt_assert(nullptr == TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()),
+        "The route between %s and %s already exists (Rq: routes are symmetrical by default).", src,dst);
+
+  /* Add the route to the base */
+  TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id()) = newExtendedRoute(hierarchy_, route, 1);
+  xbt_dynar_shrink(TO_ROUTE_FULL(src_net_elm->id(), dst_net_elm->id())->link_list, 0);
 
   if (route->symmetrical == TRUE) {
     if (route->gw_dst && route->gw_src) {

diff --git a/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh b/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh
index 2403429..efcf85d 100644 (file)
--- a/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh
+++ b/teshsuite/simdag/platforms/bogus_two_hosts_asymetric.tesh
@@ -1,11 +1,9 @@
 ! expect signal SIGABRT
 $ ${bindir:=.}/flatifier bogus_two_hosts_asymetric.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n"
 > [  0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks.
-> [  0.000000] [0:maestro@] Parse error at bogus_two_hosts_asymetric.xml:24: A route between "alice" and "bob" already exists with a different content. If you are trying to define a reverse route, you must set the symmetrical=no attribute to your routes tags.
-> [  0.000000] [0:maestro@] Exiting now
+> [  0.000000] [0:maestro@] Error while loading bogus_two_hosts_asymetric.xml: The route between alice and bob already exists (Rq: routes are symmetrical by default).
 
 ! expect signal SIGABRT
 $ ${bindir:=.}/flatifier bogus_two_hosts_asymetric-2.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n"
 > [  0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks.
-> [  0.000000] [0:maestro@] Parse error at bogus_two_hosts_asymetric-2.xml:26: A route between "alice" and "bob" already exists with a different content. If you are trying to define a reverse route, you must set the symmetrical=no attribute to your routes tags.
-> [  0.000000] [0:maestro@] Exiting now
+> [  0.000000] [0:maestro@] Error while loading bogus_two_hosts_asymetric-2.xml: The route between alice and bob already exists (Rq: routes are symmetrical by default).