coll/ml: fix coverity issues

hjelmn · hjelmn · commit c8b077f232ce · 2016-03-18T10:11:16.000-06:00
Fix CID 715744 (#1 of 1): Logically dead code (DEADCODE): Fix CID 715745 (#1 of 1): Logically dead code (DEADCODE): The free of scratch_num in either place is defensive programming. Instead of removing the free the conditional around the free has been removed to quiet the warning. Fix CID 715753 (#1 of 1): Dereference after null check (FORWARD_NULL): Fix CID 715778 (#1 of 1): Dereference before null check (REVERSE_INULL): Fixed the conditional to check for collective_alg != NULL instead of collective_alg->functions != NULL. Fix CID 715749 (#1 of 4): Explicit null dereferenced (FORWARD_NULL): Updated code to ensure that none of the parse functions are reached with a non-NULL value. Fix CID 715746 (#1 of 1): Logically dead code (DEADCODE): Removed dead code. Fix CID 715768 (#1 of 1): Resource leak (RESOURCE_LEAK): Fix CID 715769 (#2 of 2): Resource leak (RESOURCE_LEAK): Fix CID 715772 (#1 of 1): Resource leak (RESOURCE_LEAK): Move free calls to before error checks to cleanup leak in error paths. Fix CID 741334 (#1 of 1): Explicit null dereferenced (FORWARD_NULL): Added a check to ensure temp is not dereferenced if it is NULL. Fix CID 1196605 (#1 of 1): Bad bit shift operation (BAD_SHIFT): Fixed overflow in calculation by replacing int mask with 1ul. Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
diff --git a/ompi/mca/coll/ml/coll_ml_config.c b/ompi/mca/coll/ml/coll_ml_config.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) 2013-2014 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2013-2016 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -440,6 +440,9 @@ static int parse_line(section_config_t *section)
     if (COLL_ML_CONFIG_PARSE_SINGLE_WORD == val ||
         COLL_ML_CONFIG_PARSE_VALUE == val) {
         value = strdup(coll_ml_config_yytext);
+        if (NULL == value) {
+            return OMPI_ERR_OUT_OF_RESOURCE;
+        }
 
         /* Now we need to see the newline */
         val = coll_ml_config_yylex();
@@ -458,48 +461,29 @@ static int parse_line(section_config_t *section)
              COLL_ML_CONFIG_PARSE_NEWLINE != val) {
         ML_ERROR(("Line %d, expected new line or end of line",
                     coll_ml_config_yynewlines));
-        ret = OMPI_ERROR;
-        goto Error;
+        return OMPI_ERROR;
+    } else {
+        ML_ERROR(("Line %d malformed", coll_ml_config_yynewlines));
+        return OMPI_ERROR;
     }
 
     /* Line parsing is done, read the values */
     if (!strcasecmp(key_buffer, "algorithm")) {
         ret = parse_algorithm_key(section, value);
-        if (OMPI_SUCCESS != ret) {
-            goto Error;
-        }
-    }
-
-    else if (!strcasecmp(key_buffer, "threshold")) {
+    } else if (!strcasecmp(key_buffer, "threshold")) {
         ret = parse_threshold_key(section, value);
-        if (OMPI_SUCCESS != ret) {
-            goto Error;
-        }
-    }
-
-    else if (!strcasecmp(key_buffer, "hierarchy")) {
+    } else if (!strcasecmp(key_buffer, "hierarchy")) {
         ret = parse_hierarchy_key(section, value);
-        if (OMPI_SUCCESS != ret) {
-            goto Error;
-        }
-    }
-
-    else if (!strcasecmp(key_buffer, "fragmentation")) {
+    } else if (!strcasecmp(key_buffer, "fragmentation")) {
         ret = parse_fragmentation_key(section, value);
-        if (OMPI_SUCCESS != ret) {
-            goto Error;
-        }
     /* Failed to parse the key */
     } else {
         ML_ERROR(("Line %d, unknown key %s",
                     coll_ml_config_yynewlines, key_buffer));
     }
 
     /* All done */
-Error:
-    if (NULL != value) {
-        free(value);
-    }
+    free(value);
 
     return ret;
 }
diff --git a/ompi/mca/coll/ml/coll_ml_hier_algorithms_setup.c b/ompi/mca/coll/ml/coll_ml_hier_algorithms_setup.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      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2014-2016 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -253,32 +253,19 @@ int ml_coll_up_and_down_hier_setup(mca_coll_ml_module_t *ml_module,
     topo_info->hierarchical_algorithms[collective]->n_buffers = 1;
 
     /* Release temporary memories */
-    if (NULL != scratch_indx) {
-        free(scratch_indx);
-    }
-
-    if (NULL != scratch_num) {
-       free(scratch_num);
-    }
+    free(scratch_indx);
+    free(scratch_num);
 
     return OMPI_SUCCESS;
 
 Error:
-    if (NULL != collective_alg->functions) {
-       free(collective_alg->functions);
-    }
-
     if (NULL != collective_alg) {
-       free(collective_alg);
-    }
-
-    if (NULL != scratch_indx) {
-        free(scratch_indx);
+        free(collective_alg->functions);
     }
 
-    if (NULL != scratch_num) {
-        free(scratch_num);
-    }
+    free(collective_alg);
+    free(scratch_indx);
+    free(scratch_num);
 
     return ret;
 }
@@ -527,13 +514,8 @@ int ml_coll_barrier_constant_group_data_setup(
     return OMPI_SUCCESS;
 
 Const_Data_Setup_Error:
-    if (NULL != scratch_indx) {
-        free(scratch_indx);
-    }
-
-    if (NULL != scratch_num) {
-        free(scratch_num);
-    }
+    free(scratch_indx);
+    free(scratch_num);
 
     return ret;
 }
diff --git a/ompi/mca/coll/ml/coll_ml_module.c b/ompi/mca/coll/ml/coll_ml_module.c
@@ -2,7 +2,7 @@
 /*
  * Copyright (c) 2009-2013 Oak Ridge National Laboratory.  All rights reserved.
  * Copyright (c) 2009-2012 Mellanox Technologies.  All rights reserved.
- * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2012-2016 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * Copyright (c) 2013-2014 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2014      Research Organization for Information Science
@@ -476,14 +476,13 @@ static int calculate_buffer_header_size(mca_coll_ml_module_t *ml_module)
                             MPI_INT, ompi_comm_rank(ml_module->comm),
                             MPI_MAX, comm_size,
                             ranks_in_comm, ml_module->comm);
-
+    free(ranks_in_comm);
     if (OPAL_UNLIKELY(OMPI_SUCCESS != rc)) {
         ML_ERROR(("comm_allreduce_pml failed."));
         return OMPI_ERROR;
     }
 
     ml_module->data_offset = (uint32_t) data_offset;
-    free(ranks_in_comm);
 
     ML_VERBOSE(10, ("The offset is %d", ml_module->data_offset));
 
@@ -1105,6 +1104,10 @@ static int get_new_subgroup_data (int32_t *all_selected, int size_of_all_selecte
 
         (*sub_group_meta_data)[sg_id].index_of_first_element=offset;
 
+        if ((*sub_group_meta_data)[sg_id].n_ranks && NULL == temp) {
+            return OMPI_ERROR;
+        }
+
         for( array_id=0 ; array_id < (*sub_group_meta_data)[sg_id].n_ranks ;
              array_id++ ) {
             (*list_of_ranks_in_all_subgroups)[offset+array_id]=
@@ -1317,14 +1320,12 @@ static int ml_module_set_small_msg_thresholds(mca_coll_ml_module_t *ml_module)
                             BCOL_NUM_OF_FUNCTIONS, MPI_INT,
                             ompi_comm_rank(ml_module->comm), MPI_MIN,
                             comm_size, ranks_in_comm, ml_module->comm);
-
+    free(ranks_in_comm);
     if (OPAL_UNLIKELY(OMPI_SUCCESS != rc)) {
         ML_ERROR(("comm_allreduce_pml failed."));
         return OMPI_ERROR;
     }
 
-    free(ranks_in_comm);
-
     return OMPI_SUCCESS;
 }
 
@@ -2319,9 +2320,6 @@ static int mca_coll_ml_fill_in_route_tab(mca_coll_ml_topology_t *topo, ompi_comm
 
     int32_t **route_table = NULL;
     int32_t *all_reachable_ranks = NULL;
-
-    struct ompi_proc_t **sbgp_procs = NULL;
-
     mca_sbgp_base_module_t *sbgp_group = NULL;
     comm_size = ompi_comm_size(comm);
 
@@ -2500,13 +2498,7 @@ static int mca_coll_ml_fill_in_route_tab(mca_coll_ml_topology_t *topo, ompi_comm
         free(route_table);
     }
 
-    if (NULL != sbgp_procs) {
-        free(sbgp_procs);
-    }
-
-    if (NULL != all_reachable_ranks) {
-        free(all_reachable_ranks);
-    }
+    free(all_reachable_ranks);
 
     return rc;
 }
@@ -2668,6 +2660,7 @@ static int check_for_max_supported_ml_modules(struct ompi_communicator_t *comm)
                              1 , MPI_INT, ompi_comm_rank(comm),
                              MPI_MIN, ompi_comm_size(comm), comm_ranks,
                              comm);
+    free(comm_ranks);
     if (OMPI_SUCCESS != ret) {
         ML_ERROR(("comm_allreduce - failed to collect max_comm data"));
         return ret;
@@ -2680,8 +2673,6 @@ static int check_for_max_supported_ml_modules(struct ompi_communicator_t *comm)
         --cs->max_comm;
     }
 
-    free(comm_ranks);
-
     return OMPI_SUCCESS;
 }
 
diff --git a/ompi/mca/coll/ml/coll_ml_select.c b/ompi/mca/coll/ml/coll_ml_select.c
@@ -2,7 +2,7 @@
 /*
  * Copyright (c) 2009-2013 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) 2013-2014 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2014      Research Organization for Information Science
@@ -127,8 +127,7 @@ static int add_to_invoke_table(mca_bcol_base_module_t *bcol_module,
     struct mca_bcol_base_coll_fn_invoke_attributes_t *inv_attribs = NULL;
     int bcoll_type, data_src_type, waiting_semantic;
     int range_min,range_max;
-    int i=0,j=0,k=0,mask=1;
-
+    int i=0,j=0,k=0;
 
 
     if((NULL == fn_filtered->inv_attr)||(NULL == fn_filtered->comm_attr)) {
@@ -148,7 +147,7 @@ static int add_to_invoke_table(mca_bcol_base_module_t *bcol_module,
     for (j=0; j<OMPI_OP_NUM_OF_TYPES; j++) {
         for (k=0; k<OMPI_DATATYPE_MAX_PREDEFINED; k++) {
 
-            if ((inv_attribs->datatype_bitmap & (mask << k)) && (inv_attribs->op_types_bitmap & (mask << j))){
+            if ((inv_attribs->datatype_bitmap & (1ul << k)) && (inv_attribs->op_types_bitmap & (1ul << j))){
 
                for (i=range_min; i<=range_max; i++) {
                     bcol_module->filtered_fns_table[data_src_type][waiting_semantic][bcoll_type][i][j][k]
