Skip to content

Commit 3540b65

Browse files
hjelmnhjelmn
committed
bcol: fix coverity issues
Fix CID 1269976 (#1 of 1): Unused value (UNUSED_VALUE): Fix CID 1269979 (#1 of 1): Unused value (UNUSED_VALUE): Removed unused variables k_temp1 and k_temp2. Fix CID 1269981 (#1 of 1): Unused value (UNUSED_VALUE): Fix CID 1269974 (#1 of 1): Unused value (UNUSED_VALUE): Removed gotos and use the matched flags to decide whether to return. Fix CID 715755 (#1 of 1): Dereference null return value (NULL_RETURNS): This was also a leak. The items on cs->ctl_structures are allocated using OBJ_NEW so they mist be released using OBJ_RELEASE not OBJ_DESTRUCT. Replaced the loop with OPAL_LIST_DESTRUCT(). Fix CID 715776 (#1 of 1): Dereference before null check (REVERSE_INULL): Rework error path to remove REVERSE_INULL. Also added a free to an error path where it was missing. Fix CID 1196603 (#1 of 1): Bad bit shift operation (BAD_SHIFT): Fix CID 1196601 (#1 of 1): Bad bit shift operation (BAD_SHIFT): Both of these are false positives but it is still worthwhile to fix so they no longer appear. The loop conditional has been updated to use radix_mask_pow instead of radix_mask to quiet these issues. Fix CID 1269804 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS): In general close (-1) is safe but coverity doesn’t like it. Reworked the error path for open to not try to close (-1). Signed-off-by: Nathan Hjelm <[email protected]>
1 parent c8b077f commit 3540b65

File tree

5 files changed

+44
-48
lines changed

5 files changed

+44
-48
lines changed

‎ompi/mca/bcol/basesmuma/bcol_basesmuma_component.c‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
/*
33
* Copyright (c) 2009-2012 Oak Ridge National Laboratory. All rights reserved.
44
* Copyright (c) 2009-2012 Mellanox Technologies. All rights reserved.
5-
* Copyright (c) 2014-2015 Los Alamos National Security, LLC. All rights
5+
* Copyright (c) 2014-2016 Los Alamos National Security, LLC. All rights
66
* reserved.
77
* $COPYRIGHT$
88
*
@@ -271,12 +271,7 @@ static int basesmuma_close(void)
271271
mca_bcol_basesmuma_component_t *cs = &mca_bcol_basesmuma_component;
272272

273273
/* gvm Leak FIX */
274-
while(!opal_list_is_empty(&(cs->ctl_structures))) {
275-
opal_list_item_t *item;
276-
item = opal_list_remove_first(&(cs->ctl_structures));
277-
OBJ_DESTRUCT(item);
278-
}
279-
OBJ_DESTRUCT(&(cs->ctl_structures));
274+
OPAL_LIST_DESTRUCT (&cs->ctl_structures);
280275

281276
/* deregister the progress function */
282277
ret=opal_progress_unregister(bcol_basesmuma_progress);
@@ -331,7 +326,7 @@ int mca_bcol_basesmuma_init_query(bool enable_progress_threads,
331326
int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs)
332327
{
333328
/* local variables */
334-
int name_length, ret;
329+
int name_length, ret = OMPI_SUCCESS;
335330
size_t ctl_length;
336331
char *name;
337332
size_t page_size = getpagesize ();
@@ -347,6 +342,7 @@ int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs
347342
}
348343
/* make sure name is not too long */
349344
if ( OPAL_PATH_MAX < (name_length-1) ) {
345+
free (name);
350346
return OMPI_ERROR;
351347
}
352348

@@ -371,21 +367,14 @@ int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs
371367
if( !cs->sm_ctl_structs) {
372368
opal_output (ompi_bcol_base_framework.framework_output,
373369
"In mca_bcol_basesmuma_allocate_sm_ctl_memory failed to allocathe backing file %s\n", name);
374-
ret=OMPI_ERR_OUT_OF_RESOURCE;
375-
goto Error;
370+
ret = OMPI_ERR_OUT_OF_RESOURCE;
376371
}
377372

378373
/* free the memory allocated by asprintf for the file name -
379374
* in mca_base_smcm_mem_reg this name is copied into a new
380375
* memory location */
381-
free(name);
376+
free (name);
382377

383378
/* successful return */
384-
return OMPI_SUCCESS;
385-
386-
Error:
387-
if(name) {
388-
free(name);
389-
}
390379
return ret;
391380
}

‎ompi/mca/bcol/basesmuma/bcol_basesmuma_rk_barrier.c‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2009-2012 Oak Ridge National Laboratory. All rights reserved.
34
* Copyright (c) 2009-2012 Mellanox Technologies. All rights reserved.
5+
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
6+
* reserved.
47
* $COPYRIGHT$
58
*
69
* Additional copyrights may follow
@@ -72,7 +75,7 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
7275
int pow_k, tree_order;
7376
int max_requests = 0; /* important to initialize this */
7477

75-
int matched = 0;
78+
bool matched;
7679
int64_t sequence_number=input_args->sequence_num;
7780
int my_rank = bcol_module->super.sbgp_partner_module->my_index;
7881

@@ -128,10 +131,8 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
128131
src = exchange_node->rank_extra_sources_array[0];
129132
peer_ctl_pointer = data_buffs[src].ctl_struct;
130133

131-
for( i = 0; i < cm->num_to_probe && (0 == matched); i++ ) {
134+
for( i = 0; i < cm->num_to_probe ; i++ ) {
132135
if(IS_PEER_READY(peer_ctl_pointer, ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
133-
matched = 1;
134-
135136
goto FINISHED;
136137
}
137138

@@ -148,20 +149,21 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
148149
peer_ctl_pointer = data_buffs[src].ctl_struct;
149150

150151
/* probe for extra rank's arrival */
151-
for( i = 0; i < cm->num_to_probe && ( 0 == matched); i++) {
152+
for( i = 0, matched = false ; i < cm->num_to_probe && !matched ; i++) {
152153
if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
153-
matched = 1;
154-
/* copy it in */
155-
goto MAIN_PHASE;
154+
/* copy it in */
155+
matched = true;
156+
break;
156157
}
157158
}
158-
*status = ready_flag;
159-
*iteration = -1;
160-
return BCOL_FN_STARTED;
161159

160+
if (!matched) {
161+
*status = ready_flag;
162+
*iteration = -1;
163+
return BCOL_FN_STARTED;
164+
}
162165
}
163166

164-
MAIN_PHASE:
165167
/* bump the ready flag */
166168
ready_flag++;
167169

@@ -189,12 +191,11 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
189191
* better temporal locality, this comes at a cost to asynchronicity
190192
* but should get better cache performance
191193
*/
192-
matched = 0;
193-
for( probe = 0; probe < cm->num_to_probe && (0 == matched); probe++){
194+
for( probe = 0; probe < cm->num_to_probe ; probe++){
194195
if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
195-
matched = 1;
196196
/* set this request's bit */
197197
*active_requests ^= (1<<j);
198+
break;
198199
}
199200
}
200201
}
@@ -261,7 +262,7 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
261262
int pow_k, tree_order;
262263
int bcol_id = (int) bcol_module->super.bcol_id;
263264

264-
int matched = 0;
265+
bool matched;
265266
int64_t sequence_number=input_args->sequence_num;
266267
int my_rank = bcol_module->super.sbgp_partner_module->my_index;
267268

@@ -311,10 +312,8 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
311312
src = exchange_node->rank_extra_sources_array[0];
312313
peer_ctl_pointer = data_buffs[src].ctl_struct;
313314

314-
for( i = 0; i < cm->num_to_probe && (0 == matched); i++ ) {
315+
for( i = 0; i < cm->num_to_probe ; i++ ) {
315316
if(IS_PEER_READY(peer_ctl_pointer, ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
316-
matched = 1;
317-
318317
goto FINISHED;
319318
}
320319

@@ -330,21 +329,21 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
330329
peer_ctl_pointer = data_buffs[src].ctl_struct;
331330

332331
/* probe for extra rank's arrival */
333-
for( i = 0; i < cm->num_to_probe && ( 0 == matched); i++) {
332+
for( i = 0, matched = false ; i < cm->num_to_probe && !matched ; i++) {
334333
if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
335-
matched = 1;
334+
matched = true;
336335
/* bump the flag */
337336
ready_flag++;
338337
*iteration = 0;
339-
goto MAIN_PHASE;
338+
break;
340339
}
341340
}
342-
return BCOL_FN_STARTED;
343341

342+
if (!matched) {
343+
return BCOL_FN_STARTED;
344+
}
344345
}
345346

346-
MAIN_PHASE:
347-
348347
/* start the recursive k - ing phase */
349348
for( *iter=*iteration; *iter < pow_k; (*iter)++) {
350349
/* I am ready at this level */
@@ -369,12 +368,11 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
369368
/* I am putting the probe loop as the inner most loop to achieve
370369
* better temporal locality
371370
*/
372-
matched = 0;
373-
for( probe = 0; probe < cm->num_to_probe && (0 == matched); probe++){
371+
for( probe = 0; probe < cm->num_to_probe ; probe++){
374372
if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
375-
matched = 1;
376373
/* flip the request's bit */
377374
*active_requests ^= (1<<j);
375+
break;
378376
}
379377
}
380378
}

‎ompi/mca/bcol/basesmuma/bcol_basesmuma_smcm.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* Copyright (c) 2009-2012 Oak Ridge National Laboratory. All rights reserved.
55
* Copyright (c) 2009-2012 Mellanox Technologies. All rights reserved.
6-
* Copyright (c) 2012-2014 Los Alamos National Security, LLC. All rights
6+
* Copyright (c) 2012-2016 Los Alamos National Security, LLC. All rights
77
* reserved.
88
* Copyright (c) 2014 Intel, Inc. All rights reserved.
99
* Copyright (c) 2014-2015 Research Organization for Information Science
@@ -371,7 +371,10 @@ bcol_basesmuma_smcm_mmap_t *bcol_basesmuma_smcm_mem_reg(void *in_ptr,
371371
if (fd < 0) {
372372
opal_output (ompi_bcol_base_framework.framework_output, "basesmuma shared memory allocation open failed with errno: %d",
373373
errno);
374-
} else if (0 != ftruncate(fd,length)) {
374+
return NULL;
375+
}
376+
377+
if (0 != ftruncate(fd,length)) {
375378
opal_output (ompi_bcol_base_framework.framework_output, "basesmuma shared memory allocation ftruncate failed with errno: %d",
376379
errno);
377380
} else {

‎ompi/mca/bcol/ptpcoll/bcol_ptpcoll_bcast.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2009-2012 Oak Ridge National Laboratory. All rights reserved.
34
* Copyright (c) 2009-2012 Mellanox Technologies. All rights reserved.
5+
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
6+
* reserved.
47
* $COPYRIGHT$
58
*
69
* Additional copyrights may follow

‎ompi/mca/bcol/ptpcoll/bcol_ptpcoll_bcast.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2009-2012 Oak Ridge National Laboratory. All rights reserved.
34
* Copyright (c) 2009-2012 Mellanox Technologies. All rights reserved.
5+
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
6+
* reserved.
47
* $COPYRIGHT$
58
*
69
* Additional copyrights may follow
@@ -66,7 +69,7 @@ do {
6669
int dst_boundary_rank; \
6770
int radix_mask = radix_mask_pow >= 0 ? 1 << radix_mask_pow : 0; \
6871
\
69-
while(radix_mask > 0) { \
72+
while(radix_mask_pow >= 0) { \
7073
/* For each level of tree, do sends */ \
7174
dst = my_group_index ^ radix_mask; \
7275
comm_dst = group_list[dst]; \

0 commit comments

Comments
 (0)