VPP-189 - Coverity OVERRUN error in port-range

[vpp.git] / vnet / vnet / ip / ip4_mtrie.c

diff --git a/vnet/vnet/ip/ip4_mtrie.c b/vnet/vnet/ip/ip4_mtrie.c
index ed4a0d9..006610a 100644 (file)
--- a/vnet/vnet/ip/ip4_mtrie.c
+++ b/vnet/vnet/ip/ip4_mtrie.c
@@ -191,7 +191,8 @@ set_ply_with_more_specific_leaf (ip4_fib_mtrie_t * m,
       /* Replace less specific terminal leaves with new leaf. */
       else if (new_leaf_dst_address_bits >= ply->dst_address_bits_of_leaves[i])
        {
-         ply->leaves[i] = new_leaf;
+          __sync_val_compare_and_swap (&ply->leaves[i], old_leaf, new_leaf);
+          ASSERT(ply->leaves[i] == new_leaf);
          ply->dst_address_bits_of_leaves[i] = new_leaf_dst_address_bits;
          ply->n_non_empty_leafs += ip4_fib_mtrie_leaf_is_empty (old_leaf);
        }
@@ -240,7 +241,9 @@ set_leaf (ip4_fib_mtrie_t * m,
              if (old_leaf_is_terminal)
                {
                  old_ply->dst_address_bits_of_leaves[i] = a->dst_address_length;
-                 old_ply->leaves[i] = new_leaf;
+                  __sync_val_compare_and_swap (&old_ply->leaves[i], old_leaf,
+                                               new_leaf);
+                  ASSERT(old_ply->leaves[i] == new_leaf);
                  old_ply->n_non_empty_leafs += ip4_fib_mtrie_leaf_is_empty (old_leaf);
                  ASSERT (old_ply->n_non_empty_leafs <= ARRAY_LEN (old_ply->leaves));
                }
@@ -274,7 +277,9 @@ set_leaf (ip4_fib_mtrie_t * m,
          /* Refetch since ply_create may move pool. */
          old_ply = pool_elt_at_index (m->ply_pool, old_ply_index);
 
-         old_ply->leaves[dst_byte] = new_leaf;
+          __sync_val_compare_and_swap (&old_ply->leaves[dst_byte], old_leaf,
+                                       new_leaf);
+          ASSERT(old_ply->leaves[dst_byte] == new_leaf);
          old_ply->dst_address_bits_of_leaves[dst_byte] = 0;
 
          old_ply->n_non_empty_leafs -= ip4_fib_mtrie_leaf_is_non_empty (old_leaf);
@@ -298,7 +303,7 @@ unset_leaf (ip4_fib_mtrie_t * m,
 {
   ip4_fib_mtrie_leaf_t old_leaf, del_leaf;
   i32 n_dst_bits_next_plies;
-  uword i, n_dst_bits_this_ply, old_leaf_is_terminal;
+  i32 i, n_dst_bits_this_ply, old_leaf_is_terminal;
   u8 dst_byte;
 
   ASSERT (a->dst_address_length > 0 && a->dst_address_length <= 32);
@@ -431,10 +436,26 @@ maybe_remap_leaf (ip_lookup_main_t * lm, ip4_fib_mtrie_leaf_t * p)
       if (m)
        {
          was_remapped_to_empty_leaf = m == ~0;
+
+          /*
+           * The intent of the original form - which dates to 2013 or
+           * earlier - is not obvious. Here's the original:
+           * 
+           * if (was_remapped_to_empty_leaf)
+           *   p[0] = (was_remapped_to_empty_leaf
+           *           ? IP4_FIB_MTRIE_LEAF_EMPTY
+           *           : ip4_fib_mtrie_leaf_set_adj_index (m - 1));
+           *
+           * Notice the outer "if (was_remapped_to_empty_leaf)"
+           * means that p[0] is always set to IP4_FIB_MTRIE_LEAF_EMPTY,
+           * and is otherwise left intact.
+           * 
+           * It seems unlikely that the adjacency mapping scheme
+           * works in detail. Coverity correctly complains that the 
+           * else-case of the original ternary expression is dead code.
+           */
          if (was_remapped_to_empty_leaf)
-           p[0] = (was_remapped_to_empty_leaf
-                   ? IP4_FIB_MTRIE_LEAF_EMPTY
-                   : ip4_fib_mtrie_leaf_set_adj_index (m - 1));
+            p[0] = IP4_FIB_MTRIE_LEAF_EMPTY;
        }
     }
   return was_remapped_to_empty_leaf;