From 6f6f34f620a18638eb820a9ab3d8a93e350d722a Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 8 Aug 2016 13:05:31 -0400 Subject: VPP-189 Clean up more coverity warnings Change-Id: If66713d79c545c762c385faf08cc809347741152 Signed-off-by: Dave Barach --- vnet/vnet/ip/ip4_mtrie.c | 2 +- vnet/vnet/ip/ip6_neighbor.c | 8 ++++---- vnet/vnet/map/ip4_map_t.c | 3 ++- vnet/vnet/sr/sr_replicate.c | 3 ++- vnet/vnet/unix/tuntap.c | 8 +++++--- vpp/vpp-api/api.c | 11 ++++++++--- vppinfra/Makefile.am | 21 --------------------- vppinfra/vppinfra/elf.c | 10 +++++++--- vppinfra/vppinfra/hash.c | 6 +++--- vppinfra/vppinfra/heap.h | 2 +- vppinfra/vppinfra/serialize.c | 5 +++-- vppinfra/vppinfra/slist.c | 3 ++- vppinfra/vppinfra/timer.c | 2 +- 13 files changed, 39 insertions(+), 45 deletions(-) diff --git a/vnet/vnet/ip/ip4_mtrie.c b/vnet/vnet/ip/ip4_mtrie.c index 0badde769c2..006610a0f4e 100644 --- a/vnet/vnet/ip/ip4_mtrie.c +++ b/vnet/vnet/ip/ip4_mtrie.c @@ -303,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); diff --git a/vnet/vnet/ip/ip6_neighbor.c b/vnet/vnet/ip/ip6_neighbor.c index 2593cb46156..1dd09c11052 100644 --- a/vnet/vnet/ip/ip6_neighbor.c +++ b/vnet/vnet/ip/ip6_neighbor.c @@ -1437,7 +1437,8 @@ icmp6_router_advertisement(vlib_main_t * vm, /* check for MTU or prefix options or .. */ u8 * opt_hdr = (u8 *)(h0 + 1); - while( options_len0 > 0) + while( options_len0 > 0 && + opt_hdr < p0->data + p0->current_data) { icmp6_neighbor_discovery_option_header_t *o0 = ( icmp6_neighbor_discovery_option_header_t *)opt_hdr; int opt_len = o0->n_data_u64s << 3; @@ -1666,11 +1667,10 @@ ip6_neighbor_sw_interface_add_del (vnet_main_t * vnm, a->min_delay_between_radv = MIN_DELAY_BETWEEN_RAS; a->max_delay_between_radv = MAX_DELAY_BETWEEN_RAS; a->max_rtr_default_lifetime = MAX_DEF_RTR_LIFETIME; - a->seed = random_default_seed(); + a->seed = (u32) (clib_cpu_time_now() & 0xFFFFFFFF); /* for generating random interface ids */ - a->randomizer = 0x1119194911191949; - a->randomizer = random_u64 ((u32 *)&a->randomizer); + a->randomizer = random_u64 (&a->seed); a->initial_adverts_count = MAX_INITIAL_RTR_ADVERTISEMENTS ; a->initial_adverts_sent = a->initial_adverts_count-1; diff --git a/vnet/vnet/map/ip4_map_t.c b/vnet/vnet/map/ip4_map_t.c index f4bae608e4d..6c3a80cc5f3 100644 --- a/vnet/vnet/map/ip4_map_t.c +++ b/vnet/vnet/map/ip4_map_t.c @@ -376,7 +376,8 @@ _ip4_map_t_icmp (map_domain_t * d, vlib_buffer_t * p, u8 * error) } else { - ASSERT (0); // We had a port from that, so it is udp or tcp or ICMP + /* To shut up Coverity */ + os_panic(); } //FIXME: Security check with the port found in the inner packet diff --git a/vnet/vnet/sr/sr_replicate.c b/vnet/vnet/sr/sr_replicate.c index b3fc3edaa4f..8a51fdb882d 100644 --- a/vnet/vnet/sr/sr_replicate.c +++ b/vnet/vnet/sr/sr_replicate.c @@ -357,7 +357,8 @@ sr_replicate_node_fn (vlib_main_t * vm, (hdr_ip0->payload_length); } tr->next_index = next_index; - memcpy (tr->sr, hdr_sr0, sizeof (tr->sr)); + if (hdr_sr0) + memcpy (tr->sr, hdr_sr0, sizeof (tr->sr)); } } diff --git a/vnet/vnet/unix/tuntap.c b/vnet/vnet/unix/tuntap.c index f24d71eee8a..967c0aca4ac 100644 --- a/vnet/vnet/unix/tuntap.c +++ b/vnet/vnet/unix/tuntap.c @@ -428,8 +428,9 @@ tuntap_exit (vlib_main_t * vm) clib_unix_warning ("TUNSETPERSIST"); close(tm->dev_tap_fd); if (tm->dev_net_tun_fd >= 0) - close(tm->dev_net_tun_fd); - close (sfd); + close(tm->dev_net_tun_fd); + if (sfd >= 0) + close (sfd); return 0; } @@ -792,7 +793,8 @@ tuntap_ip6_add_del_interface_address (ip6_main_t * im, if (ioctl (sockfd, SIOCSIFADDR, &ifr6) < 0) clib_unix_warning ("set address"); - close (sockfd); + if (sockfd >= 0) + close (sockfd); } else { diff --git a/vpp/vpp-api/api.c b/vpp/vpp-api/api.c index bed280a1e61..18063acc7d5 100644 --- a/vpp/vpp-api/api.c +++ b/vpp/vpp-api/api.c @@ -2588,9 +2588,11 @@ vl_api_sw_interface_clear_stats_t_handler (vl_api_sw_interface_clear_stats_t * vlib_combined_counter_main_t *cm; static vnet_main_t **my_vnet_mains; int i, j, n_counters; - int rv = 0; + if (mp->sw_if_index != ~0) + VALIDATE_SW_IF_INDEX(mp); + vec_reset_length (my_vnet_mains); for (i = 0; i < vec_len (vnet_mains); i++) @@ -2632,6 +2634,8 @@ vl_api_sw_interface_clear_stats_t_handler (vl_api_sw_interface_clear_stats_t * } } + BAD_SW_IF_INDEX_LABEL; + REPLY_MACRO (VL_API_SW_INTERFACE_CLEAR_STATS_REPLY); } @@ -7768,6 +7772,8 @@ vl_api_ipfix_dump_t_handler (vl_api_ipfix_dump_t * mp) vl_api_ipfix_details_t *rmp; q = vl_api_client_index_to_input_queue (mp->client_index); + if (!q) + return; rmp = vl_msg_api_alloc (sizeof (*rmp)); memset (rmp, 0, sizeof (*rmp)); @@ -7919,8 +7925,7 @@ static void } // Validate mask_length - if (mask_length < 0 || - (is_ipv6 && mask_length > 128) || (!is_ipv6 && mask_length > 32)) + if ((is_ipv6 && mask_length > 128) || (!is_ipv6 && mask_length > 32)) { rv = VNET_API_ERROR_ADDRESS_LENGTH_MISMATCH; goto reply; diff --git a/vppinfra/Makefile.am b/vppinfra/Makefile.am index a70e7ce2084..a9de5ad03b3 100644 --- a/vppinfra/Makefile.am +++ b/vppinfra/Makefile.am @@ -35,11 +35,8 @@ TESTS += test_bihash_template \ test_macros \ test_md5 \ test_mheap \ - test_pfhash \ - test_phash \ test_pool_iterate \ test_ptclosure \ - test_qhash \ test_random \ test_random_isaac \ test_serialize \ @@ -66,11 +63,8 @@ test_longjmp_SOURCES = vppinfra/test_longjmp.c test_macros_SOURCES = vppinfra/test_macros.c test_md5_SOURCES = vppinfra/test_md5.c test_mheap_SOURCES = vppinfra/test_mheap.c -test_pfhash_SOURCES = vppinfra/test_pfhash.c -test_phash_SOURCES = vppinfra/test_phash.c test_pool_iterate_SOURCES = vppinfra/test_pool_iterate.c test_ptclosure_SOURCES = vppinfra/test_ptclosure.c -test_qhash_SOURCES = vppinfra/test_qhash.c test_random_SOURCES = vppinfra/test_random.c test_random_isaac_SOURCES = vppinfra/test_random_isaac.c test_serialize_SOURCES = vppinfra/test_serialize.c @@ -95,11 +89,8 @@ test_longjmp_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_macros_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_md5_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_mheap_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG -test_pfhash_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG -test_phash_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_pool_iterate_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_ptclosure_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG -test_qhash_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_random_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_random_isaac_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG test_socket_CPPFLAGS = $(AM_CPPFLAGS) -DCLIB_DEBUG @@ -122,11 +113,8 @@ test_longjmp_LDADD = libvppinfra.la test_macros_LDADD = libvppinfra.la test_md5_LDADD = libvppinfra.la test_mheap_LDADD = libvppinfra.la -test_pfhash_LDADD = libvppinfra.la -test_phash_LDADD = libvppinfra.la test_pool_iterate_LDADD = libvppinfra.la test_ptclosure_LDADD = libvppinfra.la -test_qhash_LDADD = libvppinfra.la test_random_LDADD = libvppinfra.la test_random_isaac_LDADD = libvppinfra.la test_serialize_LDADD = libvppinfra.la @@ -149,11 +137,8 @@ test_longjmp_LDFLAGS = -static test_macros_LDFLAGS = -static test_md5_LDFLAGS = -static test_mheap_LDFLAGS = -static -test_pfhash_LDFLAGS = -static -test_phash_LDFLAGS = -static test_pool_iterate_LDFLAGS = -static test_ptclosure_LDFLAGS = -static -test_qhash_LDFLAGS = -static test_random_LDFLAGS = -static test_random_isaac_LDFLAGS = -static test_serialize_LDFLAGS = -static @@ -206,12 +191,9 @@ nobase_include_HEADERS = \ vppinfra/mheap.h \ vppinfra/mheap_bootstrap.h \ vppinfra/os.h \ - vppinfra/pfhash.h \ - vppinfra/phash.h \ vppinfra/pipeline.h \ vppinfra/pool.h \ vppinfra/ptclosure.h \ - vppinfra/qhash.h \ vppinfra/random.h \ vppinfra/random_buffer.h \ vppinfra/random_isaac.h \ @@ -259,10 +241,7 @@ CLIB_CORE = \ vppinfra/mheap.c \ vppinfra/md5.c \ vppinfra/mem_mheap.c \ - vppinfra/pfhash.c \ - vppinfra/phash.c \ vppinfra/ptclosure.c \ - vppinfra/qhash.c \ vppinfra/random.c \ vppinfra/random_buffer.c \ vppinfra/random_isaac.c \ diff --git a/vppinfra/vppinfra/elf.c b/vppinfra/vppinfra/elf.c index 71ae1d33d70..8c5dc6c33e0 100644 --- a/vppinfra/vppinfra/elf.c +++ b/vppinfra/vppinfra/elf.c @@ -582,7 +582,7 @@ format_elf_main (u8 * s, va_list * args) s = format (s, "\nSegments: %d at file offset 0x%Lx-0x%Lx:\n", fh->segment_header_count, fh->segment_header_file_offset, - fh->segment_header_file_offset + fh->segment_header_count * fh->segment_header_size); + (u64) fh->segment_header_file_offset + fh->segment_header_count * fh->segment_header_size); s = format (s, "%U\n", format_elf_segment, 0); vec_foreach (h, copy) @@ -1827,8 +1827,12 @@ clib_error_t * elf_write_file (elf_main_t * em, char * file_name) /* Finally write section headers. */ if (fseek (f, em->file_header.section_header_file_offset, SEEK_SET) < 0) - return clib_error_return_unix (0, "fseek 0x%Lx", em->file_header.section_header_file_offset); - + { + fclose(f); + return clib_error_return_unix + (0, "fseek 0x%Lx", em->file_header.section_header_file_offset); + } + vec_foreach (s, em->sections) { elf64_section_header_t h; diff --git a/vppinfra/vppinfra/hash.c b/vppinfra/vppinfra/hash.c index 86231b4db5e..c4d724935a5 100644 --- a/vppinfra/vppinfra/hash.c +++ b/vppinfra/vppinfra/hash.c @@ -346,7 +346,7 @@ set_indirect_is_user (void * v, else { log2_bytes = 1 + hash_pair_log2_bytes (h); - q = clib_mem_alloc (1 << log2_bytes); + q = clib_mem_alloc (1ULL << log2_bytes); } clib_memcpy (q, &p->direct, hash_pair_bytes (h)); @@ -388,7 +388,7 @@ set_indirect (void * v, hash_pair_indirect_t * pi, uword key, log2_bytes = indirect_pair_get_log2_bytes (pi); new_len = len + 1; - if (new_len * hash_pair_bytes (h) > (1 << log2_bytes)) + if (new_len * hash_pair_bytes (h) > (1ULL << log2_bytes)) { pi->pairs = clib_mem_realloc (pi->pairs, 1 << (log2_bytes + 1), @@ -649,7 +649,7 @@ void * _hash_create (uword elts, hash_t * h_user) /* Size of hash is power of 2 >= ELTS and larger than number of bits in is_user bitmap elements. */ elts = clib_max (elts, BITS (h->is_user[0])); - elts = 1 << max_log2 (elts); + elts = 1ULL << max_log2 (elts); log2_pair_size = 1; if (h_user) diff --git a/vppinfra/vppinfra/heap.h b/vppinfra/vppinfra/heap.h index c6605dac5f0..604187c00ea 100644 --- a/vppinfra/vppinfra/heap.h +++ b/vppinfra/vppinfra/heap.h @@ -194,7 +194,7 @@ uword heap_bytes (void * v); always_inline void * _heap_new (u32 len, u32 n_elt_bytes) { - void * v = _vec_resize (0, len, len*n_elt_bytes, + void * v = _vec_resize (0, len, (uword) len*n_elt_bytes, sizeof (heap_header_t), HEAP_DATA_ALIGN); heap_header (v)->elt_bytes = n_elt_bytes; diff --git a/vppinfra/vppinfra/serialize.c b/vppinfra/vppinfra/serialize.c index 4025b704700..9c6987dd0e9 100644 --- a/vppinfra/vppinfra/serialize.c +++ b/vppinfra/vppinfra/serialize.c @@ -281,7 +281,8 @@ unserialize_vector_ha (serialize_main_t * m, unserialize_integer (m, &l, sizeof (l)); if (l > max_length) serialize_error (&m->header, clib_error_create ("bad vector length %d", l)); - p = v = _vec_resize (0, l, l*elt_bytes, header_bytes, /* align */ align); + p = v = _vec_resize (0, l, (uword) l*elt_bytes, header_bytes, + /* align */ align); while (l != 0) { @@ -407,7 +408,7 @@ unserialize_pool_helper (serialize_main_t * m, return 0; } - v = _vec_resize (0, l, l*elt_bytes, sizeof (p[0]), align); + v = _vec_resize (0, l, (uword) l*elt_bytes, sizeof (p[0]), align); p = pool_header (v); vec_unserialize (m, &p->free_indices, unserialize_vec_32); diff --git a/vppinfra/vppinfra/slist.c b/vppinfra/vppinfra/slist.c index 19302b69aaa..435a026a462 100644 --- a/vppinfra/vppinfra/slist.c +++ b/vppinfra/vppinfra/slist.c @@ -299,7 +299,8 @@ u8 * format_slist (u8 * s, va_list *args) for (i = 0; i < vec_len (head_elt->n.nexts); i++) { - s = format (s, "level %d: %d elts\n", i, sl->occupancy[i]); + s = format (s, "level %d: %d elts\n", i, + sl->occupancy ? sl->occupancy[i] : 0); if (verbose && head_elt->n.nexts[i] != (u32)~0) { diff --git a/vppinfra/vppinfra/timer.c b/vppinfra/vppinfra/timer.c index 2b2c958729d..2c71417f289 100644 --- a/vppinfra/vppinfra/timer.c +++ b/vppinfra/vppinfra/timer.c @@ -158,8 +158,8 @@ void timer_call (timer_func_t * func, any arg, f64 dt) /* Initialize time_resolution before first call to timer_interrupt */ time_resolution = 0.75 / (f64) HZ; + memset (&sa, 0, sizeof (sa)); sa.sa_handler = timer_interrupt; - sa.sa_flags = 0; if (sigaction (TIMER_SIGNAL, &sa, 0) < 0) clib_panic ("sigaction"); -- cgit 1.2.3-korg