From: Neale Ranns <nranns@cisco.com>
Date: Mon, 30 Jan 2017 14:44:58 +0000 (-0800)
Subject: MFIB Coverity warnings. The lock macro is functionally equivalent but more expressive... 
X-Git-Tag: v17.04-rc1~291
X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=05b2bf2e31a90d97425230dcbf2bca6df54519ff;p=vpp.git

MFIB Coverity warnings. The lock macro is functionally equivalent but more expressive (and might appease coverity)

Change-Id: Ie3c9b2896a487a0302903bfbdd6348f6f091c67d
Signed-off-by: Neale Ranns <nranns@cisco.com>
---

diff --git a/src/vat/api_format.c b/src/vat/api_format.c
index d4834b70708..26df1aff4df 100644
--- a/src/vat/api_format.c
+++ b/src/vat/api_format.c
@@ -6706,9 +6706,6 @@ api_ip_mroute_add_del (vat_main_t * vam)
   S;
   /* Wait for a reply... */
   W;
-
-  /* Return the good/bad news */
-  return (vam->retval);
 }
 
 static int
@@ -16546,7 +16543,10 @@ static void
   vat_json_object_add_uint (node, "src-if-index", sw_if_index_from);
   vat_json_object_add_string_copy (node, "src-if-name", sw_if_from_name);
   vat_json_object_add_uint (node, "dst-if-index", sw_if_index_to);
-  vat_json_object_add_string_copy (node, "dst-if-name", sw_if_to_name);
+  if (0 != sw_if_to_name)
+    {
+      vat_json_object_add_string_copy (node, "dst-if-name", sw_if_to_name);
+    }
   vat_json_object_add_uint (node, "state", mp->state);
 }
 
diff --git a/src/vnet/mfib/mfib_signal.c b/src/vnet/mfib/mfib_signal.c
index 9f6205de419..cd486da32f8 100644
--- a/src/vnet/mfib/mfib_signal.c
+++ b/src/vnet/mfib/mfib_signal.c
@@ -68,6 +68,28 @@ mfib_signal_module_init (void)
     mfib_signal_list_init();
 }
 
+static inline void
+mfib_signal_lock_aquire (void)
+{
+    while (__sync_lock_test_and_set (&mfib_signal_pending.mip_lock, 1))
+        ;
+}
+
+static inline void
+mfib_signal_lock_release (void)
+{
+    mfib_signal_pending.mip_lock = 0;
+}
+
+#define MFIB_SIGNAL_CRITICAL_SECTION(_body) \
+{                                           \
+    mfib_signal_lock_aquire();              \
+    do {                                    \
+        _body;                              \
+    } while (0);                            \
+    mfib_signal_lock_release();             \
+}
+
 int
 mfib_signal_send_one (struct _unix_shared_memory_queue *q,
                       u32 context)
@@ -77,13 +99,11 @@ mfib_signal_send_one (struct _unix_shared_memory_queue *q,
     /*
      * with the lock held, pop a signal from the q.
      */
-    while (__sync_lock_test_and_set (&mfib_signal_pending.mip_lock, 1))
-        ;
-    {
+    MFIB_SIGNAL_CRITICAL_SECTION(
+    ({
         li = clib_dlist_remove_head(mfib_signal_dlist_pool,
                                     mfib_signal_pending.mip_head);
-    }
-    mfib_signal_pending.mip_lock = 0;
+    }));
 
     if (~0 != li)
     {
@@ -106,13 +126,11 @@ mfib_signal_send_one (struct _unix_shared_memory_queue *q,
         /*
          * with the lock held, return the resoruces of the signals posted
          */
-        while (__sync_lock_test_and_set(&mfib_signal_pending.mip_lock, 1))
-            ;
-        {
+        MFIB_SIGNAL_CRITICAL_SECTION(
+        ({
             pool_put_index(mfib_signal_pool, si);
             pool_put_index(mfib_signal_dlist_pool, li);
-        }
-        mfib_signal_pending.mip_lock = 0;
+        }));
 
         return (1);
     }
@@ -128,9 +146,8 @@ mfib_signal_push (const mfib_entry_t *mfe,
     dlist_elt_t *elt;
     u32 si, li;
 
-    while (__sync_lock_test_and_set (&mfib_signal_pending.mip_lock, 1))
-        ;
-    {
+    MFIB_SIGNAL_CRITICAL_SECTION(
+    ({
         pool_get(mfib_signal_pool, mfs);
         pool_get(mfib_signal_dlist_pool, elt);
 
@@ -143,8 +160,7 @@ mfib_signal_push (const mfib_entry_t *mfe,
         clib_dlist_addhead(mfib_signal_dlist_pool,
                            mfib_signal_pending.mip_head,
                            li);
-    }
-    mfib_signal_pending.mip_lock = 0;
+    }));
 
     mfs->mfs_entry = mfib_entry_get_index(mfe);
     mfs->mfs_itf = mfib_itf_get_index(mfi);
@@ -179,9 +195,8 @@ mfib_signal_remove_itf (const mfib_itf_t *mfi)
         /*
          * it's in the pending q
          */
-        while (__sync_lock_test_and_set (&mfib_signal_pending.mip_lock, 1))
-            ;
-        {
+        MFIB_SIGNAL_CRITICAL_SECTION(
+        ({
             dlist_elt_t *elt;
 
             /*
@@ -194,8 +209,6 @@ mfib_signal_remove_itf (const mfib_itf_t *mfi)
             elt = pool_elt_at_index(mfib_signal_dlist_pool, li);
             pool_put_index(mfib_signal_pool, elt->value);
             pool_put(mfib_signal_dlist_pool, elt);
-        }
-
-        mfib_signal_pending.mip_lock = 0;
+        }));
     }
 }
diff --git a/src/vnet/mfib/mfib_test.c b/src/vnet/mfib/mfib_test.c
index 8735bfa73fc..8082a6bb606 100644
--- a/src/vnet/mfib/mfib_test.c
+++ b/src/vnet/mfib/mfib_test.c
@@ -235,6 +235,7 @@ mfib_test_entry (fib_node_index_t fei,
     const replicate_t *rep;
     mfib_prefix_t pfx;
     va_list ap;
+    int res;
 
     va_start(ap, n_buckets);
 
@@ -253,12 +254,11 @@ mfib_test_entry (fib_node_index_t fei,
                       "%U links to %U",
                       format_mfib_prefix, &pfx,
                       format_dpo_id, &mfe->mfe_rep, 0);
-        return (!0);
+        res = !0;
     }
     else
     {
         dpo_id_t tmp = DPO_INVALID;
-        int res;
 
         mfib_entry_contribute_forwarding(
             fei,
@@ -274,9 +274,11 @@ mfib_test_entry (fib_node_index_t fei,
         res = mfib_test_validate_rep_v(rep, n_buckets, ap);
 
         dpo_reset(&tmp);
-
-        return (res);
     }
+
+    va_end(ap);
+
+    return (res);
 }
 
 static int