From 3eac4b0c9ad5a40855855b2d58c20e277ed1d7b6 Mon Sep 17 00:00:00 2001 From: KAWASHIMA Takahiro Date: Mon, 31 Jul 2017 18:57:59 +0900 Subject: [PATCH] communicator: Refine `ompi_comm_set` error check The `ompi_comm_set` function never sets `NULL` to its first argument `ncomm`. So `NULL` check is unnecessary in its callers. Furthermore, `NULL` check may obscure a real return code when an error occurs if the variable is initialized to a `NULL` value. Also, `NULL` check is added in the `ompi_comm_set` function to avoid segmentation fault in an out-of-memory condition. Signed-off-by: KAWASHIMA Takahiro --- ompi/communicator/comm.c | 26 ++++++-------------------- ompi/dpm/dpm.c | 3 +-- ompi/mpi/c/intercomm_create.c | 4 ---- ompi/mpi/c/intercomm_merge.c | 4 ---- 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c index 0c23d51585..04dfa3308b 100644 --- a/ompi/communicator/comm.c +++ b/ompi/communicator/comm.c @@ -158,6 +158,9 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm, /* ompi_comm_allocate */ newcomm = OBJ_NEW(ompi_communicator_t); + if (NULL == newcomm) { + return OMPI_ERR_OUT_OF_RESOURCE; + } newcomm->super.s_info = NULL; /* fill in the inscribing hyper-cube dimensions */ newcomm->c_cube_dim = opal_cube_dim(local_size); @@ -354,11 +357,6 @@ int ompi_comm_create ( ompi_communicator_t *comm, ompi_group_t *group, goto exit; } - if ( NULL == newcomm ) { - rc = MPI_ERR_INTERN; - goto exit; - } - /* Determine context id. It is identical to f_2_c_handle */ rc = ompi_comm_nextcid (newcomp, comm, NULL, NULL, NULL, false, mode); if ( OMPI_SUCCESS != rc ) { @@ -580,10 +578,6 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key, local_group, /* local group */ remote_group); /* remote group */ - if ( NULL == newcomp ) { - rc = MPI_ERR_INTERN; - goto exit; - } if ( OMPI_SUCCESS != rc ) { goto exit; } @@ -1004,11 +998,7 @@ int ompi_comm_dup_with_info ( ompi_communicator_t * comm, opal_info_t *info, omp true, /* copy the topo */ comm->c_local_group, /* local group */ remote_group ); /* remote group */ - if ( NULL == newcomp ) { - rc = MPI_ERR_INTERN; - return rc; - } - if ( MPI_SUCCESS != rc) { + if ( OMPI_SUCCESS != rc) { return rc; } @@ -1103,7 +1093,7 @@ static int ompi_comm_idup_internal (ompi_communicator_t *comm, ompi_group_t *gro group, /* local group */ remote_group, /* remote group */ subreq); /* new subrequest */ - if (NULL == context->newcomp) { + if (OMPI_SUCCESS != rc) { ompi_comm_request_return (request); return rc; } @@ -1210,11 +1200,7 @@ int ompi_comm_create_group (ompi_communicator_t *comm, ompi_group_t *group, int true, /* copy the topo */ group, /* local group */ NULL); /* remote group */ - if ( NULL == newcomp ) { - rc = MPI_ERR_INTERN; - return rc; - } - if ( MPI_SUCCESS != rc) { + if ( OMPI_SUCCESS != rc) { return rc; } diff --git a/ompi/dpm/dpm.c b/ompi/dpm/dpm.c index 8759fd6a2b..647ec4e15b 100644 --- a/ompi/dpm/dpm.c +++ b/ompi/dpm/dpm.c @@ -467,8 +467,7 @@ int ompi_dpm_connect_accept(ompi_communicator_t *comm, int root, group, /* local group */ new_group_pointer /* remote group */ ); - if ( NULL == newcomp ) { - rc = OMPI_ERR_OUT_OF_RESOURCE; + if (OMPI_SUCCESS != rc) { goto exit; } diff --git a/ompi/mpi/c/intercomm_create.c b/ompi/mpi/c/intercomm_create.c index b899daa4c1..e8405242dc 100644 --- a/ompi/mpi/c/intercomm_create.c +++ b/ompi/mpi/c/intercomm_create.c @@ -190,10 +190,6 @@ int MPI_Intercomm_create(MPI_Comm local_comm, int local_leader, new_group_pointer /* remote group */ ); - if ( NULL == newcomp ) { - rc = MPI_ERR_INTERN; - goto err_exit; - } if ( MPI_SUCCESS != rc ) { goto err_exit; } diff --git a/ompi/mpi/c/intercomm_merge.c b/ompi/mpi/c/intercomm_merge.c index ed6ff727ce..12107764ce 100644 --- a/ompi/mpi/c/intercomm_merge.c +++ b/ompi/mpi/c/intercomm_merge.c @@ -109,10 +109,6 @@ int MPI_Intercomm_merge(MPI_Comm intercomm, int high, new_group_pointer, /* local group */ NULL /* remote group */ ); - if ( NULL == newcomp ) { - rc = MPI_ERR_INTERN; - goto exit; - } if ( MPI_SUCCESS != rc ) { goto exit; }