issue: HPCINFRA-1734 [CI] Coverity: upgrade to v2023.12

Change coverity to version 2023.12 Add move sematnics to replace some copy cases in constructers. Add move constructor/assignment to source_t Add event_data_t constructers for EV_IBVERBS/EV_RDMA_CM/EV_COMMAND Replace multi-step construction and assignment with single line to assign temporary object(rvalue),which invokes move semantics. Fix iftype_value variable type from char to int Fix proto variable type which is 16 bit positive number from short to uint16_t Guard m_error_queue with lock when checking if its empty() since its shared resource, and handle_recv_errqueue is only called in few cases when flag MSG_ERRQUEUE is set. Initalize uninitalized stats_file and m_octl insys_var class members Check unchecked gettime() function's return value Remove std::forwad<ip_address>(addr) which leaves addr with unknow state because of move semantics while addr is used afterwards Add coverity SUPRESS message to indicate FALSE POSITIVEs Signed-off-by: Bashar Abdelgafer <babdelgafer@nvidia.com>
Mellanox · Jul 8, 2024 · 3a63fce · 3a63fce
1 parent 4f9e224
commit 3a63fce
Show file tree

Hide file tree

Showing 29 changed files with 124 additions and 59 deletions.
diff --git a/contrib/jenkins_tests/cov.sh b/contrib/jenkins_tests/cov.sh
@@ -5,7 +5,7 @@ source $(dirname $0)/globals.sh
 
 echo "Checking for coverity ..."
 
-COVERITY_MODULE="${COVERITY_MODULE:-tools/cov-2021.12}"
+COVERITY_MODULE="${COVERITY_MODULE:-tools/cov-2023.12}"
 do_module "${COVERITY_MODULE}"
 
 cd "$WORKSPACE"

diff --git a/src/core/config_scanner.c b/src/core/config_scanner.c
@@ -1047,6 +1047,7 @@ YY_DECL
 
 		/* coverity[var_deref_op] */
 		yy_current_state = (yy_start);
+		//coverity[var_deref_op:FALSE] /* Turn off coverity check for null deref, as it is not null */
 		yy_current_state += YY_AT_BOL();
 yy_match:
 		do

diff --git a/src/core/dev/ring_allocation_logic.cpp b/src/core/dev/ring_allocation_logic.cpp
@@ -60,7 +60,7 @@ ring_allocation_logic::ring_allocation_logic(int ring_migration_ratio, source_t
                                              const resource_allocation_key &ring_profile)
     : m_ring_migration_ratio(ring_migration_ratio)
     , m_migration_try_count(ring_migration_ratio)
-    , m_source(source)
+    , m_source(std::move(source))
 {
     m_res_key = resource_allocation_key(ring_profile);
     m_migration_candidate = 0;
@@ -255,6 +255,7 @@ int cpu_manager::reserve_cpu_for_thread(pthread_t tid, int suggested_cpu /* = NO
             cpu = suggested_cpu;
         }
         CPU_ZERO(&cpu_set);
+        // coverity[overflow_const:FALSE] /*Turn off coverity check, cpu value is non-negative*/
         CPU_SET(cpu, &cpu_set);
         __log_dbg("Attach tid=%lu running on cpu=%d to cpu=%d", tid, sched_getcpu(), cpu);
         ret = pthread_setaffinity_np(tid, sizeof(cpu_set_t), &cpu_set);

diff --git a/src/core/dev/ring_allocation_logic.h b/src/core/dev/ring_allocation_logic.h
@@ -69,6 +69,31 @@ class source_t {
         , m_object(object)
     {
     }
+    source_t(const source_t &other)
+        : m_fd(other.m_fd)
+        , m_ip(other.m_ip)
+        , m_object(other.m_object)
+    {
+    }
+    source_t &operator=(const source_t &other)
+    {
+        if (this == &other) {
+            return *this;
+        }
+        m_fd = other.m_fd;
+        m_ip = other.m_ip;
+        m_object = other.m_object;
+
+        return *this;
+    }
+    source_t(source_t &&other) noexcept
+        : m_fd(std::move(other.m_fd))
+        , m_ip(std::move(other.m_ip))
+        , m_object(std::move(other.m_object))
+    {
+        other.m_fd = -1;
+        other.m_object = nullptr;
+    }
 };
 
 /**
@@ -121,7 +146,7 @@ class ring_allocation_logic_rx : public ring_allocation_logic {
         debug_print_type("Rx");
     }
     ring_allocation_logic_rx(source_t source, resource_allocation_key &ring_profile)
-        : ring_allocation_logic(safe_mce_sys().ring_migration_ratio_rx, source, ring_profile)
+        : ring_allocation_logic(safe_mce_sys().ring_migration_ratio_rx, std::move(source), ring_profile)
     {
         debug_print_type("Rx");
     }
@@ -135,7 +160,7 @@ class ring_allocation_logic_tx : public ring_allocation_logic {
         debug_print_type("Tx");
     }
     ring_allocation_logic_tx(source_t source, resource_allocation_key &ring_profile)
-        : ring_allocation_logic(safe_mce_sys().ring_migration_ratio_tx, source, ring_profile)
+        : ring_allocation_logic(safe_mce_sys().ring_migration_ratio_tx, std::move(source), ring_profile)
     {
         debug_print_type("Tx");
     }

diff --git a/src/core/dev/ring_simple.cpp b/src/core/dev/ring_simple.cpp
@@ -441,6 +441,7 @@ int ring_simple::socketxtreme_poll(struct xlio_socketxtreme_completion_t *xlio_c
     bool do_poll = true;
 
     if (likely(xlio_completions) && ncompletions) {
+        // coverity[missing_lock:FALSE] /*Turn off coverity missing_lock check*/
         if ((flags & SOCKETXTREME_POLL_TX) && !m_socketxtreme.ec_sock_list_start) {
             uint64_t poll_sn = 0;
             const std::lock_guard<decltype(m_lock_ring_tx)> lock(m_lock_ring_tx);

diff --git a/src/core/event/delta_timer.cpp b/src/core/event/delta_timer.cpp
@@ -57,7 +57,10 @@
 timer::timer()
 {
     m_list_head = nullptr;
-    gettime(&m_ts_last);
+    int ret = gettime(&m_ts_last);
+    if (ret) {
+        tmr_logerr("gettime() returned with value %d and error (errno %d %m)", ret, errno);
+    }
 }
 
 timer::~timer()
@@ -165,6 +168,7 @@ void timer::remove_all_timers(timer_handler *handler)
             remove_from_list(node_tmp);
             // Remove & Free node
             free(node_tmp);
+            // coverity[assigned_pointer:FALSE] /* Turn off coverity check,intended assign*/
             node_tmp = nullptr;
         } else {
             node = node->next;

diff --git a/src/core/event/event_handler_manager.cpp b/src/core/event/event_handler_manager.cpp
@@ -546,13 +546,7 @@ void event_handler_manager::priv_register_ibverbs_events(ibverbs_reg_info_t &inf
     event_handler_map_t::iterator i;
     i = m_event_handler_map.find(info.fd);
     if (i == m_event_handler_map.end()) {
-        event_data_t v = {};
-
-        v.type = EV_IBVERBS;
-        v.ibverbs_ev.fd = info.fd;
-        v.ibverbs_ev.channel = info.channel;
-
-        m_event_handler_map[info.fd] = v;
+        m_event_handler_map[info.fd] = event_data_t(EV_IBVERBS, info);
         i = m_event_handler_map.find(info.fd);
 
         priv_prepare_ibverbs_async_event_queue(i);
@@ -632,14 +626,7 @@ void event_handler_manager::priv_register_rdma_cm_events(rdma_cm_reg_info_t &inf
     event_handler_map_t::iterator iter_fd = m_event_handler_map.find(info.fd);
     if (iter_fd == m_event_handler_map.end()) {
         evh_logdbg("Adding new channel (fd %d, id %p, handler %p)", info.fd, info.id, info.handler);
-        event_data_t map_value = {};
-
-        map_value.type = EV_RDMA_CM;
-        map_value.rdma_cm_ev.n_ref_count = 1;
-        map_value.rdma_cm_ev.map_rdma_cm_id[info.id] = info.handler;
-        map_value.rdma_cm_ev.cma_channel = info.cma_channel;
-
-        m_event_handler_map[info.fd] = map_value;
+        m_event_handler_map[info.fd] = event_data_t(EV_RDMA_CM, info);
 
         update_epfd(info.fd, EPOLL_CTL_ADD, EPOLLIN | EPOLLPRI);
     } else {
@@ -707,12 +694,7 @@ void event_handler_manager::priv_register_command_events(command_reg_info_t &inf
     event_handler_map_t::iterator iter_fd = m_event_handler_map.find(info.fd);
     if (iter_fd == m_event_handler_map.end()) {
         evh_logdbg("Adding new channel (fd %d)", info.fd);
-        event_data_t map_value = {};
-
-        map_value.type = EV_COMMAND;
-        map_value.command_ev.cmd = info.cmd;
-
-        m_event_handler_map[info.fd] = map_value;
+        m_event_handler_map[info.fd] = event_data_t(EV_COMMAND, info);
         update_epfd(info.fd, EPOLL_CTL_ADD, EPOLLIN | EPOLLPRI);
     }
 }

diff --git a/src/core/event/event_handler_manager.h b/src/core/event/event_handler_manager.h
@@ -141,6 +141,34 @@ struct event_data_t {
     ibverbs_ev_t ibverbs_ev;
     rdma_cm_ev_t rdma_cm_ev;
     command_ev_t command_ev;
+    event_data_t() = default;
+    event_data_t(ev_type t, const ibverbs_reg_info_t &info)
+        : type(t)
+        , ibverbs_ev {}
+        , rdma_cm_ev {}
+        , command_ev {}
+    {
+        ibverbs_ev.fd = info.fd;
+        ibverbs_ev.channel = info.channel;
+    }
+    event_data_t(ev_type t, const rdma_cm_reg_info_t &info)
+        : type(t)
+        , ibverbs_ev {}
+        , rdma_cm_ev {}
+        , command_ev {}
+    {
+        rdma_cm_ev.n_ref_count = 1;
+        rdma_cm_ev.map_rdma_cm_id[info.id] = info.handler;
+        rdma_cm_ev.cma_channel = info.cma_channel;
+    }
+    event_data_t(ev_type t, const command_reg_info_t &info)
+        : type(t)
+        , ibverbs_ev {}
+        , rdma_cm_ev {}
+        , command_ev {}
+    {
+        command_ev.cmd = info.cmd;
+    }
 };
 
 typedef std::map<int /*fd*/, event_data_t> event_handler_map_t;

diff --git a/src/core/lwip/tcp.c b/src/core/lwip/tcp.c
@@ -438,6 +438,7 @@ u32_t tcp_update_rcv_ann_wnd(struct tcp_pcb *pcb)
                     pcb->rcv_ann_right_edge + LWIP_MIN((pcb->rcv_wnd_max / 2), pcb->mss))) {
         /* we can advertise more window */
         pcb->rcv_ann_wnd = pcb->rcv_wnd;
+        // coverity[overflow:FALSE] /* Turn off coverity check, value is non-negative */
         return new_right_edge - pcb->rcv_ann_right_edge;
     } else {
         if (TCP_SEQ_GT(pcb->rcv_nxt, pcb->rcv_ann_right_edge)) {

diff --git a/src/core/proto/flow_tuple.cpp b/src/core/proto/flow_tuple.cpp
@@ -65,7 +65,7 @@ flow_tuple::flow_tuple(const flow_tuple &ft)
 
 flow_tuple::flow_tuple(flow_tuple &&ft)
 {
-    *this = ft;
+    *this = std::move(ft);
 }
 
 flow_tuple &flow_tuple::operator=(const flow_tuple &ft)

diff --git a/src/core/proto/neighbour.h b/src/core/proto/neighbour.h
@@ -56,7 +56,7 @@
 class neigh_key {
 public:
     neigh_key(ip_addr addr, net_device_val *p_ndvl)
-        : m_ip_addrs(addr)
+        : m_ip_addrs(std::move(addr))
         , m_p_net_dev_val(p_ndvl) {};
     virtual ~neigh_key() {};
 

diff --git a/src/core/proto/neighbour_table_mgr.cpp b/src/core/proto/neighbour_table_mgr.cpp
@@ -135,16 +135,16 @@ void neigh_table_mgr::notify_cb(event *ev)
         return;
     }
 
-    ip_addr addr(ip_address(reinterpret_cast<void *>(&in), nl_info->addr_family),
-                 nl_info->addr_family);
-
     // Search for this neigh ip in cache_table
     m_lock.lock();
     net_device_val *p_ndev = g_p_net_device_table_mgr->get_net_device_val(nl_info->ifindex);
 
     // find all neigh entries with an appropriate peer_ip and net_device
     if (p_ndev) {
-        neigh_entry *p_ne = dynamic_cast<neigh_entry *>(get_entry(neigh_key(addr, p_ndev)));
+        neigh_entry *p_ne = dynamic_cast<neigh_entry *>(get_entry(
+            neigh_key(ip_addr(ip_address(reinterpret_cast<void *>(&in), nl_info->addr_family),
+                              nl_info->addr_family),
+                      p_ndev)));
         if (p_ne) {
             // Call the relevant neigh_entry to handle the event
             p_ne->handle_neigh_event(nl_ev);

diff --git a/src/core/sock/fd_collection.cpp b/src/core/sock/fd_collection.cpp
@@ -445,6 +445,7 @@ int fd_collection::add_cq_channel_fd(int cq_ch_fd, ring *p_ring)
         fdcoll_logwarn("cq channel fd already exists in fd_collection");
         m_p_cq_channel_map[cq_ch_fd] = nullptr;
         delete p_cq_ch_info;
+        // coverity[assigned_pointer] /* Turn off coverity check, intended assign*/
         p_cq_ch_info = nullptr;
     }
     BULLSEYE_EXCLUDE_BLOCK_END

diff --git a/src/core/sock/sockinfo.cpp b/src/core/sock/sockinfo.cpp
@@ -2195,11 +2195,11 @@ void sockinfo::handle_recv_errqueue(struct cmsg_state *cm_state)
 {
     mem_buf_desc_t *buff = nullptr;
 
+    m_error_queue_lock.lock();
     if (m_error_queue.empty()) {
+        m_error_queue_lock.unlock();
         return;
     }
-
-    m_error_queue_lock.lock();
     buff = m_error_queue.get_and_pop_front();
     m_error_queue_lock.unlock();
 

diff --git a/src/core/sock/sockinfo_tcp.cpp b/src/core/sock/sockinfo_tcp.cpp
@@ -2399,6 +2399,7 @@ ssize_t sockinfo_tcp::rx(const rx_call_t call_type, iovec *p_iov, ssize_t sz_iov
      * error notification without data processing
      */
     if (__msg && __msg->msg_control && (in_flags & MSG_ERRQUEUE)) {
+        // coverity[MISSING_LOCK:FALSE] /*Turn off coverity check for missing lock*/
         if (m_error_queue.empty()) {
             errno = EAGAIN;
             unlock_tcp_con();
@@ -2873,6 +2874,7 @@ int sockinfo_tcp::bind(const sockaddr *__addr, socklen_t __addrlen)
         UNLOCK_RET(-1);
     }
     m_pcb.is_ipv6 = (addr.get_sa_family() == AF_INET6);
+    // coverity[copy_assignment_call:FALSE] /*Turn off coverity check for COPY_INSTEAD_OF_MOVE*/
     m_bound = addr;
 
     if (!m_bound.is_anyaddr() &&
@@ -4047,7 +4049,6 @@ bool sockinfo_tcp::is_writeable()
     __log_funcall("--->>> tcp_sndbuf(&m_pcb)=%ld", sndbuf_available());
     return true;
 }
-
 bool sockinfo_tcp::is_errorable(int *errors)
 {
     *errors = 0;
@@ -4056,7 +4057,7 @@ bool sockinfo_tcp::is_errorable(int *errors)
         m_conn_state == TCP_CONN_RESETED || m_conn_state == TCP_CONN_FAILED) {
         *errors |= POLLHUP;
     }
-
+    // coverity[MISSING_LOCK:FALSE] /*Turn off coverity check for missing lock*/
     if ((m_conn_state == TCP_CONN_ERROR) || (!m_error_queue.empty())) {
         *errors |= POLLERR;
     }
@@ -4564,6 +4565,7 @@ int sockinfo_tcp::tcp_setsockopt(int __level, int __optname, __const void *__opt
                 ret = -1;
                 break;
             } else {
+                // coverity[copy_assignment_call:FALSE] /*Turn off coverity COPY_INSTEAD_OF_MOVE*/
                 m_so_bindtodevice_ip = addr;
 
                 si_tcp_logdbg("SOL_SOCKET, %s='%s' (%s)", setsockopt_so_opt_to_str(__optname),

diff --git a/src/core/sock/sockinfo_udp.cpp b/src/core/sock/sockinfo_udp.cpp
@@ -847,6 +847,7 @@ int sockinfo_udp::setsockopt(int __level, int __optname, __const void *__optval,
                 } else if (!get_ip_addr_from_ifname((char *)__optval, addr, m_family) ||
                            (m_family == AF_INET6 && !m_is_ipv6only &&
                             !get_ip_addr_from_ifname((char *)__optval, addr, AF_INET))) {
+                    // coverity[copy_assignment_call:FALSE] /*Turn off check COPY_INSTEAD_OF_MOVE*/
                     m_so_bindtodevice_ip = addr;
                 } else {
                     si_udp_logdbg("SOL_SOCKET, %s=\"???\" - NOT HANDLED, cannot find if_name",
@@ -1331,6 +1332,7 @@ int sockinfo_udp::setsockopt(int __level, int __optname, __const void *__optval,
             } else {
                 ip_addr src_addr {0};
                 if (get_ip_addr_from_ifindex(if_ix, src_addr, AF_INET6) == 0) {
+                    // coverity[copy_assignment_call:FALSE] /*Turn off check COPY_INSTEAD_OF_MOVE*/
                     m_mc_tx_src_ip = src_addr;
                 } else {
                     si_udp_logdbg("IPPROTO_IPV6, setsockopt(%s) will be passed to OS for "
@@ -1554,6 +1556,7 @@ int sockinfo_udp::resolve_if_ip(const int if_index, const ip_address &ip, ip_add
         } else {
             ip_addr src_addr {0};
             if (get_ip_addr_from_ifindex(if_index, src_addr, m_family) == 0) {
+                // coverity[copy_assignment_call:FALSE] /*Turn off check COPY_INSTEAD_OF_MOVE*/
                 resolved_ip = src_addr;
             } else {
                 si_udp_logdbg("Can't find interface IP of interface index %d", if_index);
@@ -2117,6 +2120,7 @@ ssize_t sockinfo_udp::tx(xlio_tx_call_attr_t &tx_arg)
                 // Fast path
                 // We found our target dst_entry object
                 m_p_last_dst_entry = p_dst_entry = dst_entry_iter->second;
+                // coverity[copy_assignment_call:FALSE] /*Turn off check COPY_INSTEAD_OF_MOVE*/
                 m_last_sock_addr = dst;
             } else {
                 // Slow path
@@ -2530,6 +2534,7 @@ bool sockinfo_udp::rx_input_cb(mem_buf_desc_t *p_desc, void *pv_fd_ready_array)
                 m_port_map.erase(std::remove(m_port_map.begin(), m_port_map.end(),
                                              m_port_map[m_port_map_index].port));
                 if (m_port_map_index) {
+                    // coverity[underflow:FALSE] /*Turn off coverity check for underflow*/
                     m_port_map_index--;
                 }
                 m_port_map_lock.unlock();

diff --git a/src/core/util/hugepage_mgr.cpp b/src/core/util/hugepage_mgr.cpp
@@ -251,6 +251,7 @@ size_t hugepage_mgr::read_meminfo(const char *tag)
         }
         infile.close();
     }
+    // coverity[return_overflow:FALSE] /*Turn off coverity check for overflow*/
     return val;
 }
 

diff --git a/src/core/util/ip_address.h b/src/core/util/ip_address.h
@@ -104,7 +104,7 @@ class ip_address {
 
     ip_address(const ip_address &addr) { *this = addr; }
 
-    ip_address(ip_address &&addr) { *this = addr; }
+    ip_address(ip_address &&addr) { *this = std::move(addr); }
 
     const std::string to_str(sa_family_t family) const
     {
@@ -315,7 +315,7 @@ class ip_addr : public ip_address {
     }
 
     ip_addr(ip_addr &&addr)
-        : ip_address(std::forward<ip_address>(addr))
+        : ip_address(addr)
         , m_family(addr.m_family)
     {
     }

diff --git a/src/core/util/sys_vars.h b/src/core/util/sys_vars.h
@@ -539,6 +539,8 @@ struct mce_sys_var {
     // prevent unautothrized creation of objects
     mce_sys_var()
         : sysctl_reader(sysctl_reader_t::instance())
+        , stats_file(nullptr)
+        , m_ioctl {}
     {
         // coverity[uninit_member]
         get_env_params();

diff --git a/src/core/util/utils.cpp b/src/core/util/utils.cpp
@@ -606,7 +606,7 @@ int get_iftype_from_ifname(const char *ifname)
     char iftype_filename[100];
     char iftype_value_str[32];
     char base_ifname[32];
-    char iftype_value = -1;
+    int iftype_value = -1;
 
     get_base_interface_name(ifname, base_ifname, sizeof(base_ifname));
     sprintf(iftype_filename, IFTYPE_PARAM_FILE, base_ifname);

diff --git a/src/core/util/xlio_stats.h b/src/core/util/xlio_stats.h
@@ -507,8 +507,10 @@ typedef struct sh_mem_t {
         global_inst_arr->init();
         mc_info.max_grp_num = 0;
         for (uint32_t i = 0; i < MC_TABLE_SIZE; i++) {
-            mc_info.mc_grp_tbl[i].mc_grp = {ip_address::any_addr(), 0};
-            mc_info.mc_grp_tbl[i].sock_num = 0;
+            // coverity[missing_lock:FALSE] /*Turn off coverity missing_lock check*/
+            auto &mc_grp_entry = mc_info.mc_grp_tbl[i];
+            mc_grp_entry.mc_grp = {ip_address::any_addr(), 0};
+            mc_grp_entry.sock_num = 0;
         }
         memset(&iomux, 0, sizeof(iomux));
         for (uint32_t i = 0; i < max_skt_inst_num; i++) {