Fix weight comparison in edge BFS

… and add a test to reproduce the issue.

Noticed and reported by Jingrong Chen (@crazyboycjr)
https://github.com/gsliepen/tinc/issues/393

diff --git a/src/graph.c b/src/graph.c
index da64909..c100a93 100644 (file)
--- a/src/graph.c
+++ b/src/graph.c
@@ -115,11 +115,13 @@ static void mst_kruskal(void) {
        }
 }
 
+// Not putting it into header, the outside world doesn't need to know about it.
+extern void sssp_bfs(void);
+
 /* Implementation of a simple breadth-first search algorithm.
    Running time: O(E)
 */
-
-static void sssp_bfs(void) {
+void sssp_bfs(void) {
        list_t *todo_list = list_alloc(NULL);
 
        /* Clear visited status on nodes */
@@ -181,7 +183,7 @@ static void sssp_bfs(void) {
 
                        // Only update nexthop if it doesn't increase the path length
 
-                       if(!e->to->status.visited || (e->to->distance == n->distance + 1 && e->weight >= e->to->prevedge->weight)) {
+                       if(!e->to->status.visited || (e->to->distance == n->distance + 1 && e->weight < e->to->prevedge->weight)) {
                                e->to->nexthop = (n->nexthop == myself) ? e->to : n->nexthop;
                        }
 

diff --git a/test/unit/meson.build b/test/unit/meson.build
index 8633c18..5200598 100644 (file)
--- a/test/unit/meson.build
+++ b/test/unit/meson.build
@@ -31,6 +31,9 @@ tests = {
     'code': 'test_random_noinit.c',
     'fail': true,
   },
+  'graph': {
+    'code': 'test_graph.c',
+  },
   'netutl': {
     'code': 'test_netutl.c',
   },

diff --git a/test/unit/test_graph.c b/test/unit/test_graph.c
new file mode 100644 (file)
index 0000000..5d72104
--- /dev/null
+++ b/test/unit/test_graph.c
@@ -0,0 +1,98 @@
+#include "unittest.h"
+#include "../../src/connection.h"
+#include "../../src/node.h"
+#include "../../src/xalloc.h"
+
+extern void sssp_bfs(void);
+
+static void connect_nodes(node_t *from, node_t *to, int weight) {
+       edge_t *direct = new_edge();
+       direct->from = from;
+       direct->to = to;
+       direct->weight = weight;
+       edge_add(direct);
+
+       edge_t *reverse = new_edge();
+       reverse->from = to;
+       reverse->to = from;
+       reverse->weight = weight;
+       edge_add(reverse);
+}
+
+static node_t *make_node(const char *name) {
+       node_t *node = new_node();
+       node->name = xstrdup(name);
+       node->status.reachable = true;
+       node_add(node);
+       return node;
+}
+
+static void test_sssp_bfs(void **state) {
+       (void)state;
+
+       node_t *mars = make_node("mars");
+       node_t *saturn = make_node("saturn");
+       node_t *neptune = make_node("neptune");
+
+       //          50            1000
+       // myself ------ mars ------------- neptune
+       //      \                             /
+       //       ----------------- saturn ----
+       //              500                10
+
+       // Upper route
+       connect_nodes(myself, mars, 50);
+       connect_nodes(mars, neptune, 1000);
+
+       // Lower route
+       connect_nodes(myself, saturn, 500);
+       connect_nodes(saturn, neptune, 10);
+
+       sssp_bfs();
+
+       assert_true(mars->status.visited);
+       assert_true(saturn->status.visited);
+       assert_true(neptune->status.visited);
+
+       assert_false(mars->status.indirect);
+       assert_false(saturn->status.indirect);
+       assert_false(neptune->status.indirect);
+
+       assert_int_equal(1, mars->distance);
+       assert_int_equal(1, saturn->distance);
+       assert_int_equal(2, neptune->distance);
+
+       assert_ptr_equal(mars, mars->nexthop);
+       assert_ptr_equal(saturn, saturn->nexthop);
+       assert_ptr_equal(saturn, neptune->nexthop);
+
+       assert_ptr_equal(lookup_edge(myself, mars), mars->prevedge);
+       assert_ptr_equal(lookup_edge(myself, saturn), saturn->prevedge);
+       assert_ptr_equal(lookup_edge(saturn, neptune), neptune->prevedge);
+
+       assert_ptr_equal(mars, mars->via);
+       assert_ptr_equal(saturn, saturn->via);
+       assert_ptr_equal(neptune, neptune->via);
+}
+
+static int setup(void **state) {
+       (void)state;
+       myself = new_node();
+       myself->name = xstrdup("myself");
+       return 0;
+}
+
+static int teardown(void **state) {
+       (void)state;
+       free_node(myself);
+       exit_nodes();
+       exit_edges();
+       return 0;
+}
+
+int main(void) {
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test_setup_teardown(test_sssp_bfs, setup, teardown),
+       };
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}