From ebf19ac5eb6cdcfa1bb90d73a5e658433a760f60 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Sat, 26 Sep 2015 11:08:51 -0600 Subject: [PATCH] osc/pt2pt: fix coveity issues Fixed CID 1269712, 1269709, 1269706, 1269703, 1269694: Logically dead code Remove extra NULL check as OMPI_OSC_PT2PT_REQUEST_ALLOC can never set the request to NULL. Fixes CID 1269668: Unchecked return value False positive. Add (void) to indicate we do not care about the return code from opal_hash_table_get_uint32. Fixes CID 1324726: Free of address-of expression Do not free lock if it was not allocated. Fixes CID 1269658: Free of address-of expression Never will happen but because op is always a built-in op there is no reason to retain/release it anyway. Signed-off-by: Nathan Hjelm --- ompi/mca/osc/pt2pt/osc_pt2pt_comm.c | 21 +++---------------- ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c | 13 ------------ ompi/mca/osc/pt2pt/osc_pt2pt_passive_target.c | 4 +++- 3 files changed, 6 insertions(+), 32 deletions(-) diff --git a/ompi/mca/osc/pt2pt/osc_pt2pt_comm.c b/ompi/mca/osc/pt2pt/osc_pt2pt_comm.c index 2f26b28c50..5bb3a070cf 100644 --- a/ompi/mca/osc/pt2pt/osc_pt2pt_comm.c +++ b/ompi/mca/osc/pt2pt/osc_pt2pt_comm.c @@ -63,9 +63,9 @@ static int ompi_osc_pt2pt_dt_send_complete (ompi_request_t *request) OBJ_RELEASE(datatype); OPAL_THREAD_LOCK(&mca_osc_pt2pt_component.lock); - opal_hash_table_get_value_uint32(&mca_osc_pt2pt_component.modules, - ompi_comm_get_cid(request->req_mpi_object.comm), - (void **) &module); + (void) opal_hash_table_get_value_uint32(&mca_osc_pt2pt_component.modules, + ompi_comm_get_cid(request->req_mpi_object.comm), + (void **) &module); OPAL_THREAD_UNLOCK(&mca_osc_pt2pt_component.lock); assert (NULL != module); @@ -619,9 +619,6 @@ int ompi_osc_pt2pt_compare_and_swap (const void *origin_addr, const void *compar /* compare-and-swaps are always request based, so that we know where to land the data */ OMPI_OSC_PT2PT_REQUEST_ALLOC(win, request); - if (NULL == request) { - return OMPI_ERR_OUT_OF_RESOURCE; - } request->type = OMPI_OSC_PT2PT_HDR_TYPE_CSWAP; request->origin_addr = origin_addr; @@ -705,9 +702,6 @@ int ompi_osc_pt2pt_rput(const void *origin_addr, int origin_count, target_count, target_dt->name, win->w_name)); OMPI_OSC_PT2PT_REQUEST_ALLOC(win, pt2pt_request); - if (NULL == pt2pt_request) { - return OMPI_ERR_OUT_OF_RESOURCE; - } /* short-circuit case */ if (0 == origin_count || 0 == target_count) { @@ -764,9 +758,6 @@ static inline int ompi_osc_pt2pt_rget_internal (void *origin_addr, int origin_co /* gets are always request based, so that we know where to land the data */ OMPI_OSC_PT2PT_REQUEST_ALLOC(win, pt2pt_request); - if (NULL == pt2pt_request) { - return OMPI_ERR_OUT_OF_RESOURCE; - } pt2pt_request->internal = release_req; @@ -914,9 +905,6 @@ int ompi_osc_pt2pt_raccumulate(const void *origin_addr, int origin_count, win->w_name)); OMPI_OSC_PT2PT_REQUEST_ALLOC(win, pt2pt_request); - if (NULL == pt2pt_request) { - return OMPI_ERR_OUT_OF_RESOURCE; - } /* short-circuit case */ if (0 == origin_count || 0 == target_count) { @@ -979,9 +967,6 @@ int ompi_osc_pt2pt_rget_accumulate_internal (const void *origin_addr, int origin /* get_accumulates are always request based, so that we know where to land the data */ OMPI_OSC_PT2PT_REQUEST_ALLOC(win, pt2pt_request); - if (OPAL_UNLIKELY(NULL == pt2pt_request)) { - return OMPI_ERR_OUT_OF_RESOURCE; - } pt2pt_request->internal = release_req; diff --git a/ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c b/ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c index 4e349518e2..6883d79a4a 100644 --- a/ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c +++ b/ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c @@ -76,10 +76,6 @@ static void osc_pt2pt_accumulate_data_destructor (osc_pt2pt_accumulate_data_t *a if (acc_data->datatype) { OBJ_RELEASE(acc_data->datatype); } - - if (acc_data->op) { - OBJ_RELEASE(acc_data->op); - } } OBJ_CLASS_DECLARATION(osc_pt2pt_accumulate_data_t); @@ -644,7 +640,6 @@ static int osc_pt2pt_accumulate_allocate (ompi_osc_pt2pt_module_t *module, int p acc_data->datatype = datatype; OBJ_RETAIN(datatype); acc_data->op = op; - OBJ_RETAIN(op); acc_data->request_count = request_count; *acc_data_out = acc_data; @@ -813,8 +808,6 @@ static int ompi_osc_pt2pt_acc_start (ompi_osc_pt2pt_module_t *module, int source ret = osc_pt2pt_accumulate_buffer (target, data, data_len, proc, acc_header->count, datatype, op); - OBJ_RELEASE(op); - ompi_osc_pt2pt_accumulate_unlock (module); return ret; @@ -890,8 +883,6 @@ static int ompi_osc_pt2pt_acc_long_start (ompi_osc_pt2pt_module_t *module, int s } } while (0); - OBJ_RELEASE(op); - if (OPAL_UNLIKELY(OMPI_SUCCESS != ret)) { ompi_osc_pt2pt_accumulate_unlock (module); } @@ -941,8 +932,6 @@ static int ompi_osc_pt2pt_gacc_start (ompi_osc_pt2pt_module_t *module, int sourc } } while (0); - OBJ_RELEASE(op); - if (OPAL_UNLIKELY(OMPI_SUCCESS != ret)) { ompi_osc_pt2pt_accumulate_unlock (module); } @@ -1022,8 +1011,6 @@ static int ompi_osc_gacc_long_start (ompi_osc_pt2pt_module_t *module, int source } } while (0); - OBJ_RELEASE(op); - if (OPAL_UNLIKELY(OMPI_SUCCESS != ret)) { ompi_osc_pt2pt_accumulate_unlock (module); } diff --git a/ompi/mca/osc/pt2pt/osc_pt2pt_passive_target.c b/ompi/mca/osc/pt2pt/osc_pt2pt_passive_target.c index 4d4db86c52..0ddc4cf326 100644 --- a/ompi/mca/osc/pt2pt/osc_pt2pt_passive_target.c +++ b/ompi/mca/osc/pt2pt/osc_pt2pt_passive_target.c @@ -332,7 +332,9 @@ static int ompi_osc_pt2pt_lock_internal (int lock_type, int target, int assert, ret = ompi_osc_pt2pt_lock_internal_execute (module, lock); if (OPAL_UNLIKELY(OMPI_SUCCESS != ret)) { OPAL_THREAD_SCOPED_LOCK(&module->lock, ompi_osc_pt2pt_module_lock_remove (module, lock)); - ompi_osc_pt2pt_sync_return (lock); + if (&module->all_sync != lock) { + ompi_osc_pt2pt_sync_return (lock); + } } return ret;