some PR feedback

Signed-off-by: Howard Pritchard <[email protected]>

Author: hppritcha

ompi/communicator/comm.c

@@ -24,7 +24,7 @@
  * Copyright (c) 2015      Mellanox Technologies. All rights reserved.
  * Copyright (c) 2017-2022 IBM Corporation.  All rights reserved.
  * Copyright (c) 2021      Nanook Consulting.  All rights reserved.
- * Copyright (c) 2018-2022 Triad National Security, LLC. All rights
+ * Copyright (c) 2018-2024 Triad National Security, LLC. All rights
  *                         reserved.
  * Copyright (c) 2023      Advanced Micro Devices, Inc. All rights reserved.
  * $COPYRIGHT$
@@ -1738,7 +1738,7 @@ int ompi_intercomm_create_from_groups (ompi_group_t *local_group, int local_lead
                                        ompi_communicator_t **newintercomm)
 {
     ompi_communicator_t *newcomp = NULL, *local_comm, *leader_comm = MPI_COMM_NULL;
-    ompi_comm_extended_cid_block_t new_block = {{0}};
+    ompi_comm_extended_cid_block_t new_block = {0};
     bool i_am_leader = local_leader == local_group->grp_my_rank;
     ompi_proc_t **rprocs;
     uint64_t data[4];

ompi/communicator/comm_cid.c

@@ -315,7 +315,7 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
     opal_process_name_t opal_proc_name;
     bool cid_base_set = false;
     char *tag = NULL;
-    size_t proc_count = 0, rproc_count = 0, cid_base = 0UL, ninfo;
+    size_t proc_count = 0, rproc_count = 0, tproc_count = 0, cid_base = 0UL, ninfo;
     int rc, leader_rank;
     pmix_proc_t *procs;
     void *grpinfo = NULL, *list = NULL;
@@ -338,13 +338,15 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
 
     grpinfo = PMIx_Info_list_start();
     if (NULL == grpinfo) {
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
 
     rc = PMIx_Info_list_add(grpinfo, PMIX_GROUP_ASSIGN_CONTEXT_ID, NULL, PMIX_BOOL);
     if (PMIX_SUCCESS != rc) {
         OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Info_list_add failed %s %d", PMIx_Error_string(rc), __LINE__));
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
 
     list = PMIx_Info_list_start();
@@ -353,32 +355,34 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
     rc = PMIx_Info_list_add(list, PMIX_GROUP_LOCAL_CID, &c_index, PMIX_SIZE);
     if (PMIX_SUCCESS != rc) {
         OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Info_list_add failed %s %d", PMIx_Error_string(rc), __LINE__));
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
 
     rc = PMIx_Info_list_convert(list, &darray);
     if (PMIX_SUCCESS != rc) {
         OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Info_list_convert failed %s %d", PMIx_Error_string(rc), __LINE__));
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
     rc = PMIx_Info_list_add(grpinfo, PMIX_GROUP_INFO, &darray, PMIX_DATA_ARRAY);
     if (PMIX_SUCCESS != rc) {
         OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Info_list_add failed %s %d", PMIx_Error_string(rc), __LINE__));
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        PMIX_DATA_ARRAY_DESTRUCT(&darray);
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
-    PMIx_Info_list_release(list);
     PMIX_DATA_ARRAY_DESTRUCT(&darray);
 
-
     rc = PMIx_Info_list_convert(grpinfo, &darray);
     if (PMIX_SUCCESS != rc) {
         OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Info_list_convert failed %s %d", PMIx_Error_string(rc), __LINE__));
-        return OMPI_ERR_OUT_OF_RESOURCE ;
+        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        goto fn_exit;
     }
 
     pinfo = (pmix_info_t*)darray.array;
     ninfo = darray.size;
-    PMIx_Info_list_release(grpinfo);
 
     proc_count = newcomm->c_local_group->grp_proc_count;
     if ( OMPI_COMM_IS_INTER (newcomm) ){
@@ -396,10 +400,11 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
         OPAL_PMIX_CONVERT_NAME(&procs[proc_count+i],&opal_proc_name);
     }
 
+    tproc_count = proc_count + rproc_count;
 
     OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "calling PMIx_Group_construct - tag %s size %ld ninfo %ld cid_base %ld\n",
-                         tag, proc_count + rproc_count, ninfo, cid_base));
-    rc = PMIx_Group_construct(tag, procs, proc_count + rproc_count, pinfo, ninfo, &results, &nresults);
+                         tag, tproc_count, ninfo, cid_base));
+    rc = PMIx_Group_construct(tag, procs, tproc_count, pinfo, ninfo, &results, &nresults);
     PMIX_DATA_ARRAY_DESTRUCT(&darray);
     if(PMIX_SUCCESS != rc) {
        char msg_string[1024];
@@ -443,7 +448,7 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
     }
 
     OPAL_OUTPUT_VERBOSE((10, ompi_comm_output, "PMIx_Group_construct - tag %s size %ld ninfo %ld cid_base %ld\n",
-                         tag, proc_count + rproc_count, ninfo, cid_base));
+                         tag, tproc_count, ninfo, cid_base));
 
     /* destruct the group */
     rc = PMIx_Group_destruct (tag, NULL, 0);
@@ -468,10 +473,18 @@ static int ompi_comm_ext_cid_new_block (ompi_communicator_t *newcomm, ompi_commu
     }
 
     if(NULL != procs) {
-        PMIX_PROC_FREE(procs, proc_count);
+        PMIX_PROC_FREE(procs, tproc_count);
         procs = NULL;
     }
 
+    if (NULL != grpinfo) {
+        PMIx_Info_list_release(grpinfo);
+    }
+
+    if (NULL != list) {
+        PMIx_Info_list_release(list);
+    }
+
     return rc;
 }
 

ompi/mca/mtl/ofi/mtl_ofi.h

@@ -591,7 +591,7 @@ ompi_mtl_ofi_send_generic(struct mca_mtl_base_module_t *mtl,
         mtl_comm = comm->c_mtl_comm;
         /*
          * if we do not have the local cid for the target receiver,
-         * retrive via the PMIx modex
+         * retrieve via the PMIx modex
          */
         if (mtl_comm->c_index_vec[dest].c_index == 0) {
             ompi_ret = ompi_comm_get_remote_cid(comm, dest, &remote_cid);

ompi/runtime/params.h

@@ -192,7 +192,7 @@ OMPI_DECLSPEC extern int ompi_mpi_event_tick_rate;
 OMPI_DECLSPEC extern bool ompi_mpi_yield_when_idle;
 
  /**
- * A integer value specifying verbosity level for communicator management
+ * An integer value specifying verbosity level for communicator management
  * subsystem.
  */
 OMPI_DECLSPEC extern int ompi_comm_verbose_level;