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 <hjelmn@lanl.gov>

ompi/mca/bcol/basesmuma/bcol_basesmuma_component.c

@@ -2,7 +2,7 @@
 /*
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
- * Copyright (c) 2014-2015 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2014-2016 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -271,12 +271,7 @@ static int basesmuma_close(void)
     mca_bcol_basesmuma_component_t *cs = &mca_bcol_basesmuma_component;
 
     /* gvm Leak FIX */
-    while(!opal_list_is_empty(&(cs->ctl_structures))) {
-        opal_list_item_t *item;
-        item = opal_list_remove_first(&(cs->ctl_structures));
-        OBJ_DESTRUCT(item);
-    }
-    OBJ_DESTRUCT(&(cs->ctl_structures));
+    OPAL_LIST_DESTRUCT (&cs->ctl_structures);
 
     /* deregister the progress function */
     ret=opal_progress_unregister(bcol_basesmuma_progress);
@@ -331,7 +326,7 @@ int mca_bcol_basesmuma_init_query(bool enable_progress_threads,
 int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs)
 {
     /* local variables */
-    int name_length, ret;
+    int name_length, ret = OMPI_SUCCESS;
     size_t ctl_length;
     char *name;
     size_t page_size = getpagesize ();
@@ -347,6 +342,7 @@ int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs
     }
     /* make sure name is not too long */
     if ( OPAL_PATH_MAX < (name_length-1) ) {
+        free (name);
         return OMPI_ERROR;
     }
 
@@ -371,21 +367,14 @@ int mca_bcol_basesmuma_allocate_sm_ctl_memory(mca_bcol_basesmuma_component_t *cs
     if( !cs->sm_ctl_structs) {
         opal_output (ompi_bcol_base_framework.framework_output,
                      "In mca_bcol_basesmuma_allocate_sm_ctl_memory failed to allocathe backing file %s\n", name);
-        ret=OMPI_ERR_OUT_OF_RESOURCE;
-        goto Error;
+        ret = OMPI_ERR_OUT_OF_RESOURCE;
     }
 
     /* free the memory allocated by asprintf for the file name -
      * in mca_base_smcm_mem_reg this name is copied into a new
      * memory location */
-    free(name);
+    free (name);
 
     /* successful return */
-    return OMPI_SUCCESS;
-
- Error:
-    if(name) {
-        free(name);
-    }
     return ret;
 }

ompi/mca/bcol/basesmuma/bcol_basesmuma_rk_barrier.c

@@ -1,6 +1,9 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
+ * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ *                         reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -72,7 +75,7 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
     int pow_k, tree_order;
     int max_requests = 0; /* important to initialize this */
 
-    int matched = 0;
+    bool matched;
     int64_t sequence_number=input_args->sequence_num;
     int my_rank = bcol_module->super.sbgp_partner_module->my_index;
 
@@ -128,10 +131,8 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
         src = exchange_node->rank_extra_sources_array[0];
         peer_ctl_pointer = data_buffs[src].ctl_struct;
 
-        for( i = 0; i < cm->num_to_probe && (0 == matched); i++ ) {
+        for( i = 0; i < cm->num_to_probe ; i++ ) {
             if(IS_PEER_READY(peer_ctl_pointer, ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                matched = 1;
-
                 goto FINISHED;
             }
 
@@ -148,20 +149,21 @@ int bcol_basesmuma_k_nomial_barrier_init(bcol_function_args_t *input_args,
         peer_ctl_pointer = data_buffs[src].ctl_struct;
 
         /* probe for extra rank's arrival */
-        for( i = 0; i < cm->num_to_probe && ( 0 == matched); i++) {
+        for( i = 0, matched = false ; i < cm->num_to_probe && !matched  ; i++) {
             if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                matched = 1;
-                /* copy it in */
-                goto MAIN_PHASE;
+              /* copy it in */
+              matched = true;
+              break;
             }
         }
-        *status = ready_flag;
-        *iteration = -1;
-        return BCOL_FN_STARTED;
 
+        if (!matched) {
+          *status = ready_flag;
+          *iteration = -1;
+          return BCOL_FN_STARTED;
+        }
     }
 
-MAIN_PHASE:
     /* bump the ready flag */
     ready_flag++;
 
@@ -189,12 +191,11 @@ MAIN_PHASE:
                 * better temporal locality, this comes at a cost to asynchronicity
                 * but should get better cache performance
                 */
-                matched = 0;
-                for( probe = 0; probe < cm->num_to_probe && (0 == matched); probe++){
+                for( probe = 0; probe < cm->num_to_probe ; probe++){
                     if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                        matched = 1;
                         /* set this request's bit */
                         *active_requests ^= (1<<j);
+                        break;
                     }
                 }
             }
@@ -261,7 +262,7 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
     int pow_k, tree_order;
     int bcol_id = (int) bcol_module->super.bcol_id;
 
-    int matched = 0;
+    bool matched;
     int64_t sequence_number=input_args->sequence_num;
     int my_rank = bcol_module->super.sbgp_partner_module->my_index;
 
@@ -311,10 +312,8 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
         src = exchange_node->rank_extra_sources_array[0];
         peer_ctl_pointer = data_buffs[src].ctl_struct;
 
-        for( i = 0; i < cm->num_to_probe && (0 == matched); i++ ) {
+        for( i = 0; i < cm->num_to_probe ; i++ ) {
             if(IS_PEER_READY(peer_ctl_pointer, ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                matched = 1;
-
                 goto FINISHED;
             }
 
@@ -330,21 +329,21 @@ int bcol_basesmuma_k_nomial_barrier_progress(bcol_function_args_t *input_args,
         peer_ctl_pointer = data_buffs[src].ctl_struct;
 
         /* probe for extra rank's arrival */
-        for( i = 0; i < cm->num_to_probe && ( 0 == matched); i++) {
+        for( i = 0, matched = false ; i < cm->num_to_probe && !matched ; i++) {
             if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                matched = 1;
+                matched = true;
                 /* bump the flag */
                 ready_flag++;
                 *iteration = 0;
-                goto MAIN_PHASE;
+                break;
             }
         }
-        return BCOL_FN_STARTED;
 
+        if (!matched) {
+          return BCOL_FN_STARTED;
+        }
     }
 
-MAIN_PHASE:
-
     /* start the recursive k - ing phase */
     for( *iter=*iteration; *iter < pow_k; (*iter)++) {
         /* I am ready at this level */
@@ -369,12 +368,11 @@ MAIN_PHASE:
                 /* I am putting the probe loop as the inner most loop to achieve
                  * better temporal locality
                  */
-                matched = 0;
-                for( probe = 0; probe < cm->num_to_probe && (0 == matched); probe++){
+                for( probe = 0; probe < cm->num_to_probe ; probe++){
                     if(IS_PEER_READY(peer_ctl_pointer,ready_flag, sequence_number, BARRIER_RKING_FLAG, bcol_id)){
-                        matched = 1;
                         /* flip the request's bit */
                         *active_requests ^= (1<<j);
+                        break;
                     }
                 }
             }

ompi/mca/bcol/basesmuma/bcol_basesmuma_smcm.c

@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
- * Copyright (c) 2012-2014 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2012-2016 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * Copyright (c) 2014      Intel, Inc.  All rights reserved.
  * 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,
     if (fd < 0) {
         opal_output (ompi_bcol_base_framework.framework_output, "basesmuma shared memory allocation open failed with errno: %d",
                     errno);
-    } else if (0 != ftruncate(fd,length)) {
+        return NULL;
+    }
+
+    if (0 != ftruncate(fd,length)) {
         opal_output (ompi_bcol_base_framework.framework_output, "basesmuma shared memory allocation ftruncate failed with errno: %d",
                     errno);
     } else {

ompi/mca/bcol/ptpcoll/bcol_ptpcoll_bcast.c

@@ -1,6 +1,9 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
+ * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ *                         reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow

ompi/mca/bcol/ptpcoll/bcol_ptpcoll_bcast.h

@@ -1,6 +1,9 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2009-2012 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
+ * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ *                         reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -66,7 +69,7 @@ do {
     int dst_boundary_rank;                                                                                          \
     int radix_mask = radix_mask_pow >= 0 ? 1 << radix_mask_pow : 0;                                                 \
                                                                                                                     \
-    while(radix_mask > 0) {                                                                                         \
+    while(radix_mask_pow >= 0) {                                                                                    \
         /* For each level of tree, do sends */                                                                      \
         dst = my_group_index ^ radix_mask;                                                                          \
         comm_dst = group_list[dst];                                                                                 \