aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Barach <dave@barachs.net>2020-02-17 09:13:26 -0500
committerFlorin Coras <florin.coras@gmail.com>2020-02-17 17:18:18 +0000
commit47d41ad62c5d6008e72d2e9c137cf8f49ca86353 (patch)
treea21f871a26f97c6a47fbfbae4b4d67f27c6036f5
parenta316744bc5e003d0fa4c8aff82c619b300115f02 (diff)
misc: fix coverity warnings
Add an ALWAYS_ASSERT (...) macro, to (a) shut up coverity, and (b) check the indicated condition in production images. As in: p = hash_get(...); ALWAYS_ASSERT(p) /* was ASSERT(p) */ elt = pool_elt_at_index(pool, p[0]); This may not be the best way to handle a specific case, but failure to check return values at all followed by e.g. a pointer dereference isn't ok. Type: fix Ticket: VPP-1837 Signed-off-by: Dave Barach <dave@barachs.net> Change-Id: Ia97c641cefcfb7ea7d77ea5a55ed4afea0345acb
-rw-r--r--src/plugins/gtpu/gtpu.c2
-rw-r--r--src/vnet/dpo/load_balance_map.c4
-rw-r--r--src/vnet/fib/fib_attached_export.c8
-rw-r--r--src/vnet/geneve/encap.c7
-rw-r--r--src/vnet/geneve/geneve.c2
-rw-r--r--src/vnet/ipsec/ipsec_output.c2
-rw-r--r--src/vnet/session/application_worker.c2
-rw-r--r--src/vnet/vxlan-gbp/vxlan_gbp.c2
-rw-r--r--src/vnet/vxlan-gpe/vxlan_gpe.c2
-rw-r--r--src/vnet/vxlan/vxlan.c4
-rw-r--r--src/vppinfra/error_bootstrap.h29
11 files changed, 48 insertions, 16 deletions
diff --git a/src/plugins/gtpu/gtpu.c b/src/plugins/gtpu/gtpu.c
index 905ca5e1ca5..b0d7839f519 100644
--- a/src/plugins/gtpu/gtpu.c
+++ b/src/plugins/gtpu/gtpu.c
@@ -318,7 +318,7 @@ vtep_addr_unref (ip46_address_t * ip)
uword *vtep = ip46_address_is_ip4 (ip) ?
hash_get (gtpu_main.vtep4, ip->ip4.as_u32) :
hash_get_mem (gtpu_main.vtep6, &ip->ip6);
- ASSERT (vtep);
+ ALWAYS_ASSERT (vtep);
if (--(*vtep) != 0)
return *vtep;
ip46_address_is_ip4 (ip) ?
diff --git a/src/vnet/dpo/load_balance_map.c b/src/vnet/dpo/load_balance_map.c
index bf5d1fb2943..7da360b88ac 100644
--- a/src/vnet/dpo/load_balance_map.c
+++ b/src/vnet/dpo/load_balance_map.c
@@ -274,7 +274,7 @@ load_balance_map_db_remove (load_balance_map_t *lbm)
{
p = hash_get(lb_maps_by_path_index, lbmp->lbmp_index);
- ASSERT(NULL != p);
+ ALWAYS_ASSERT(NULL != p);
fib_node_list_remove(p[0], lbmp->lbmp_sibling);
}
@@ -312,7 +312,7 @@ load_balance_map_fill (load_balance_map_t *lbm)
tmp_buckets[jj++] = bucket++;
}
}
- else
+ else
{
bucket += lbmp->lbmp_weight;
}
diff --git a/src/vnet/fib/fib_attached_export.c b/src/vnet/fib/fib_attached_export.c
index 3c9c2bffee6..4a33b8897f5 100644
--- a/src/vnet/fib/fib_attached_export.c
+++ b/src/vnet/fib/fib_attached_export.c
@@ -364,7 +364,7 @@ fib_attached_export_purge (fib_entry_t *fib_entry)
fed = fib_entry_delegate_find(export_entry,
FIB_ENTRY_DELEGATE_ATTACHED_EXPORT);
- ASSERT(NULL != fed);
+ ALWAYS_ASSERT(NULL != fed);
export = pool_elt_at_index(fib_ae_export_pool, fed->fd_index);
@@ -391,7 +391,7 @@ fib_attached_export_purge (fib_entry_t *fib_entry)
pool_put(fib_ae_import_pool, import);
fib_entry_delegate_remove(fib_entry,
FIB_ENTRY_DELEGATE_ATTACHED_IMPORT);
- }
+ }
}
void
@@ -522,7 +522,7 @@ fib_ae_import_format (fib_node_index_t impi,
s = format(s, "export-sibling:%d ", import->faei_export_sibling);
s = format(s, "exporter:%d ", import->faei_exporter);
s = format(s, "export-fib:%d ", import->faei_export_fib);
-
+
s = format(s, "import-entry:%d ", import->faei_import_entry);
s = format(s, "import-fib:%d ", import->faei_import_fib);
@@ -544,7 +544,7 @@ fib_ae_export_format (fib_node_index_t expi,
fib_ae_export_t *export;
export = pool_elt_at_index(fib_ae_export_pool, expi);
-
+
s = format(s, "\n Attached-Export:%d:[", (export - fib_ae_export_pool));
s = format(s, "export-entry:%d ", export->faee_ei);
diff --git a/src/vnet/geneve/encap.c b/src/vnet/geneve/encap.c
index af52fd24543..2859a9ae652 100644
--- a/src/vnet/geneve/encap.c
+++ b/src/vnet/geneve/encap.c
@@ -131,7 +131,7 @@ geneve_encap_inline (vlib_main_t * vm,
next0 = t0->next_dpo.dpoi_next_node;
}
- ASSERT (t0 != NULL);
+ ALWAYS_ASSERT (t0 != NULL);
vnet_buffer (b[0])->ip.adj_index[VLIB_TX] = t0->next_dpo.dpoi_index;
@@ -145,7 +145,7 @@ geneve_encap_inline (vlib_main_t * vm,
next1 = t1->next_dpo.dpoi_next_node;
}
- ASSERT (t1 != NULL);
+ ALWAYS_ASSERT (t1 != NULL);
vnet_buffer (b[1])->ip.adj_index[VLIB_TX] = t1->next_dpo.dpoi_index;
@@ -378,6 +378,9 @@ geneve_encap_inline (vlib_main_t * vm,
/* Note: change to always set next0 if it may be set to drop */
next0 = t0->next_dpo.dpoi_next_node;
}
+
+ ALWAYS_ASSERT (t0 != NULL);
+
vnet_buffer (b[0])->ip.adj_index[VLIB_TX] = t0->next_dpo.dpoi_index;
/* Apply the rewrite string. $$$$ vnet_rewrite? */
diff --git a/src/vnet/geneve/geneve.c b/src/vnet/geneve/geneve.c
index 0c551a0545e..a54a5383c46 100644
--- a/src/vnet/geneve/geneve.c
+++ b/src/vnet/geneve/geneve.c
@@ -312,7 +312,7 @@ vtep_addr_unref (ip46_address_t * ip)
uword *vtep = ip46_address_is_ip4 (ip) ?
hash_get (geneve_main.vtep4, ip->ip4.as_u32) :
hash_get_mem (geneve_main.vtep6, &ip->ip6);
- ASSERT (vtep);
+ ALWAYS_ASSERT (vtep);
if (--(*vtep) != 0)
return *vtep;
ip46_address_is_ip4 (ip) ?
diff --git a/src/vnet/ipsec/ipsec_output.c b/src/vnet/ipsec/ipsec_output.c
index 2ba965a3ae8..d09a027de29 100644
--- a/src/vnet/ipsec/ipsec_output.c
+++ b/src/vnet/ipsec/ipsec_output.c
@@ -222,7 +222,7 @@ ipsec_output_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
if (PREDICT_FALSE (last_sw_if_index != sw_if_index0))
{
uword *p = hash_get (im->spd_index_by_sw_if_index, sw_if_index0);
- ASSERT (p);
+ ALWAYS_ASSERT (p);
spd_index0 = p[0];
spd0 = pool_elt_at_index (im->spds, spd_index0);
last_sw_if_index = sw_if_index0;
diff --git a/src/vnet/session/application_worker.c b/src/vnet/session/application_worker.c
index 1db6baab74e..0b67d2922bf 100644
--- a/src/vnet/session/application_worker.c
+++ b/src/vnet/session/application_worker.c
@@ -468,7 +468,7 @@ app_worker_get_listen_segment_manager (app_worker_t * app,
{
uword *smp;
smp = hash_get (app->listeners_table, listen_session_get_handle (listener));
- ASSERT (smp != 0);
+ ALWAYS_ASSERT (smp != 0);
return segment_manager_get (*smp);
}
diff --git a/src/vnet/vxlan-gbp/vxlan_gbp.c b/src/vnet/vxlan-gbp/vxlan_gbp.c
index 80eecb46c10..9a8158e9ab0 100644
--- a/src/vnet/vxlan-gbp/vxlan_gbp.c
+++ b/src/vnet/vxlan-gbp/vxlan_gbp.c
@@ -291,7 +291,7 @@ vtep_addr_unref (ip46_address_t * ip)
uword *vtep = ip46_address_is_ip4 (ip) ?
hash_get (vxlan_gbp_main.vtep4, ip->ip4.as_u32) :
hash_get_mem (vxlan_gbp_main.vtep6, &ip->ip6);
- ASSERT (vtep);
+ ALWAYS_ASSERT (vtep);
if (--(*vtep) != 0)
return *vtep;
ip46_address_is_ip4 (ip) ?
diff --git a/src/vnet/vxlan-gpe/vxlan_gpe.c b/src/vnet/vxlan-gpe/vxlan_gpe.c
index 86157c0b519..e5ff4f74295 100644
--- a/src/vnet/vxlan-gpe/vxlan_gpe.c
+++ b/src/vnet/vxlan-gpe/vxlan_gpe.c
@@ -404,7 +404,7 @@ vtep_addr_unref (ip46_address_t * ip)
uword *vtep = ip46_address_is_ip4 (ip) ?
hash_get (vxlan_gpe_main.vtep4, ip->ip4.as_u32) :
hash_get_mem (vxlan_gpe_main.vtep6, &ip->ip6);
- ASSERT (vtep);
+ ALWAYS_ASSERT (vtep);
if (--(*vtep) != 0)
return *vtep;
ip46_address_is_ip4 (ip) ?
diff --git a/src/vnet/vxlan/vxlan.c b/src/vnet/vxlan/vxlan.c
index 2b852511325..32647496a76 100644
--- a/src/vnet/vxlan/vxlan.c
+++ b/src/vnet/vxlan/vxlan.c
@@ -311,7 +311,7 @@ vtep_addr_unref (ip46_address_t * ip)
uword *vtep = ip46_address_is_ip4 (ip) ?
hash_get (vxlan_main.vtep4, ip->ip4.as_u32) :
hash_get_mem (vxlan_main.vtep6, &ip->ip6);
- ASSERT (vtep);
+ ALWAYS_ASSERT (vtep);
if (--(*vtep) != 0)
return *vtep;
ip46_address_is_ip4 (ip) ?
@@ -337,7 +337,7 @@ mcast_shared_get (ip46_address_t * ip)
{
ASSERT (ip46_address_is_multicast (ip));
uword *p = hash_get_mem (vxlan_main.mcast_shared, ip);
- ASSERT (p);
+ ALWAYS_ASSERT (p);
mcast_shared_t ret = {.as_u64 = *p };
return ret;
}
diff --git a/src/vppinfra/error_bootstrap.h b/src/vppinfra/error_bootstrap.h
index 1cd91058574..8dbbc7f359d 100644
--- a/src/vppinfra/error_bootstrap.h
+++ b/src/vppinfra/error_bootstrap.h
@@ -79,6 +79,35 @@ do { \
} \
} while (0)
+/*
+ * This version always generates code, and has a Coverity-specific
+ * version to stop Coverity complaining about
+ * ALWAYS_ASSERT(p != 0); p->member...
+ */
+
+#ifndef __COVERITY__
+#define ALWAYS_ASSERT(truth) \
+do { \
+ if (PREDICT_FALSE(!(truth))) \
+ { \
+ _clib_error (CLIB_ERROR_ABORT, 0, 0, \
+ "%s:%d (%s) assertion `%s' fails", \
+ __FILE__, \
+ (uword) __LINE__, \
+ clib_error_function, \
+ # truth); \
+ } \
+} while (0)
+#else /* __COVERITY__ */
+#define ALWAYS_ASSERT(truth) \
+do { \
+ if (PREDICT_FALSE(!(truth))) \
+ { \
+ abort(); \
+ } \
+} while (0)
+#endif /* __COVERITY */
+
#if defined(__clang__)
#define STATIC_ASSERT(truth,...)
#else