Merge pull request #11028 from hoopoepg/topic/fixed-coverity-issues

OSC/UCX: suppressed some coverity issues

Author: janjust

ompi/mca/osc/ucx/osc_ucx_comm.c

@@ -269,17 +269,22 @@ static inline int get_dynamic_win_info(uint64_t remote_addr,
     uint64_t win_count;
     int insert = -1;
     int ret;
+    int ret_unlock;
 
     /* We need to lock dyn-lock even if the process has an exclusive lock.
      * Remote process protects its window attach/detach operations with a
      * dynamic lock */
     ret = ompi_osc_ucx_dynamic_lock(module, target);
     if (ret != OPAL_SUCCESS) {
-        ret = OMPI_ERROR;
-        goto cleanup;
+        return OMPI_ERROR;
     }
 
     temp_buf = calloc(remote_state_len, 1);
+    if (NULL == temp_buf) {
+        ret = OMPI_ERR_OUT_OF_RESOURCE;
+        goto cleanup;
+    }
+
     ret = opal_common_ucx_wpmem_putget(module->state_mem, OPAL_COMMON_UCX_GET, target,
                                        (void *)((intptr_t)temp_buf),
                                        remote_state_len, remote_state_addr, ep);
@@ -316,17 +321,20 @@ static inline int get_dynamic_win_info(uint64_t remote_addr,
     _mem_record_t *mem_rec = NULL;
     ret = opal_tsd_tracked_key_get(&module->mem->tls_key, (void **) &mem_rec);
     if (OPAL_SUCCESS != ret) {
+        ret = OMPI_ERROR;
         goto cleanup;
     }
 
      if (mem_rec == NULL) {
         OSC_UCX_GET_DEFAULT_EP(ep, module, target);
         ret = opal_common_ucx_tlocal_fetch_spath(module->mem, target, ep);
         if (OPAL_SUCCESS != ret) {
+            ret = OMPI_ERROR;
             goto cleanup;
         }
         ret = opal_tsd_tracked_key_get(&module->mem->tls_key, (void **) &mem_rec);
         if (OPAL_SUCCESS != ret) {
+            ret = OMPI_ERROR;
             goto cleanup;
         }
 
@@ -361,9 +369,9 @@ static inline int get_dynamic_win_info(uint64_t remote_addr,
     free(temp_buf);
 
     /* unlock the dynamic lock */
-    ompi_osc_ucx_dynamic_unlock(module, target);
-
-    return ret;
+    ret_unlock = ompi_osc_ucx_dynamic_unlock(module, target);
+    /* ignore unlock result in case of error */
+    return (OMPI_SUCCESS != ret) ? ret : ret_unlock;
 }
 
 static inline
@@ -1615,7 +1623,7 @@ void ompi_osc_ucx_req_completion(void *request) {
         assert(req->phase != ACC_INIT);
         void *free_addr = NULL;
         bool release_lock = false;
-        ptrdiff_t temp_lb, temp_extent;
+        ptrdiff_t temp_extent, temp_lb;
         const void *origin_addr = req->origin_addr;
         int origin_count = req->origin_count;
         struct ompi_datatype_t *origin_dt = req->origin_dt;
@@ -1786,7 +1794,7 @@ void ompi_osc_ucx_req_completion(void *request) {
             module->state_mem->skip_periodic_flush = false;
         }
     }
+    assert(module->ctx->num_incomplete_req_ops > 0);
     OSC_UCX_DECREMENT_OUTSTANDING_NB_OPS(module);
     ompi_request_complete(&(ucx_req->super.super), true);
-    assert(module->ctx->num_incomplete_req_ops >= 0);
 }

ompi/mca/osc/ucx/osc_ucx_component.c

@@ -784,6 +784,9 @@ static int component_select(struct ompi_win_t *win, void **base, size_t size, in
     case MPI_WIN_FLAVOR_SHARED:
         mem_type = OPAL_COMMON_UCX_MEM_MAP;
         break;
+    default:
+        ret = OMPI_ERR_BAD_PARAM;
+        goto error;
     }
     ret = opal_common_ucx_wpmem_create(module->ctx, mem_base, module->size,
                                      mem_type, &exchange_len_info,
@@ -942,14 +945,14 @@ int ompi_osc_ucx_dynamic_lock(ompi_osc_ucx_module_t *module, int target) {
     uint64_t remote_addr = (module->state_addrs)[target] + OSC_UCX_STATE_DYNAMIC_LOCK_OFFSET;
     ucp_ep_h *ep;
     OSC_UCX_GET_DEFAULT_EP(ep, module, target);
-    int ret = OMPI_SUCCESS;
+    int ret;
 
     for (;;) {
         ret = opal_common_ucx_wpmem_cmpswp(module->state_mem,
                                         TARGET_LOCK_UNLOCKED, TARGET_LOCK_EXCLUSIVE,
                                         target, &result_value, sizeof(result_value),
                                         remote_addr, ep);
-        if (ret != OMPI_SUCCESS) {
+        if (OPAL_SUCCESS != ret) {
             OSC_UCX_VERBOSE(1, "opal_common_ucx_mem_cmpswp failed: %d", ret);
             return OMPI_ERROR;
         }
@@ -960,18 +963,18 @@ int ompi_osc_ucx_dynamic_lock(ompi_osc_ucx_module_t *module, int target) {
         opal_common_ucx_wpool_progress(mca_osc_ucx_component.wpool);
     }
 
-    return ret;
+    return OMPI_SUCCESS;
 }
 
 int ompi_osc_ucx_dynamic_unlock(ompi_osc_ucx_module_t *module, int target) {
     uint64_t result_value = -1;
     uint64_t remote_addr = (module->state_addrs)[target] + OSC_UCX_STATE_DYNAMIC_LOCK_OFFSET;
     ucp_ep_h *ep;
     OSC_UCX_GET_DEFAULT_EP(ep, module, target);
-    int ret = OMPI_SUCCESS;
+    int ret;
 
     ret = opal_common_ucx_wpmem_fence(module->mem);
-    if (ret != OMPI_SUCCESS) {
+    if (OPAL_SUCCESS != ret) {
         OSC_UCX_VERBOSE(1, "opal_common_ucx_mem_fence failed: %d", ret);
         return OMPI_ERROR;
     }
@@ -980,8 +983,12 @@ int ompi_osc_ucx_dynamic_unlock(ompi_osc_ucx_module_t *module, int target) {
                                     UCP_ATOMIC_FETCH_OP_SWAP, TARGET_LOCK_UNLOCKED,
                                     target, &result_value, sizeof(result_value),
                                     remote_addr, ep);
+    if (OPAL_SUCCESS != ret) {
+        return OMPI_ERROR;
+    }
+
     assert(result_value == TARGET_LOCK_EXCLUSIVE);
-    return ret;
+    return OMPI_SUCCESS;
 }
 
 int ompi_osc_ucx_win_attach(struct ompi_win_t *win, void *base, size_t len) {

opal/mca/common/ucx/common_ucx_wpool.c

@@ -198,7 +198,6 @@ OPAL_DECLSPEC int opal_common_ucx_wpool_init(opal_common_ucx_wpool_t *wpool)
     OBJ_DESTRUCT(&wpool->idle_workers);
     OBJ_DESTRUCT(&wpool->active_workers);
     ucp_cleanup(wpool->ucp_ctx);
-err_ucp_init:
     return rc;
 }