Skip to content

Commit a88b24c

Browse files
committed
Per comments from Jeff makes some changes to the communicator changes.
Changed: - Use ompi_mpi_group_null instead of MPI_GROUP_NULL. - Asserts don't always quiet the clang static analyser. Change them to ifs to really quite the warnings. cmr=v1.8.1:ticket=trac:4527:reviewer=jsquyres This commit was SVN r31424. The following Trac tickets were found above: Ticket 4527 --> https://svn.open-mpi.org/trac/ompi/ticket/4527
1 parent 8594f5d commit a88b24c

File tree

1 file changed

+26
-12
lines changed

1 file changed

+26
-12
lines changed

ompi/communicator/comm.c

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ int ompi_comm_set ( ompi_communicator_t **ncomm,
124124
}
125125

126126
/*
127-
* if remote_group == OMPI_GROUP_NULL, then the new communicator
127+
* if remote_group == &ompi_mpi_group_null, then the new communicator
128128
* is forced to be an inter communicator.
129129
*/
130130
int ompi_comm_set_nb ( ompi_communicator_t **ncomm,
@@ -169,7 +169,7 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm,
169169

170170
/* Set remote group and duplicate the local comm, if applicable */
171171
if (0 < remote_size) {
172-
if (NULL == remote_group || MPI_GROUP_NULL == remote_group) {
172+
if (NULL == remote_group || &ompi_mpi_group_null.group == remote_group) {
173173
ret = ompi_group_incl(oldcomm->c_remote_group, remote_size,
174174
remote_ranks, &newcomm->c_remote_group);
175175
if (OPAL_UNLIKELY(OMPI_SUCCESS != ret)) {
@@ -181,7 +181,7 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm,
181181
ompi_group_increment_proc_count(newcomm->c_remote_group);
182182
}
183183
}
184-
if (0 < remote_size || MPI_GROUP_NULL == remote_group) {
184+
if (0 < remote_size || &ompi_mpi_group_null.group == remote_group) {
185185
newcomm->c_flags |= OMPI_COMM_INTER;
186186
if ( OMPI_COMM_IS_INTRA(oldcomm) ) {
187187
ompi_comm_idup(oldcomm, &newcomm->c_local_comm, req);
@@ -273,7 +273,9 @@ int ompi_comm_create ( ompi_communicator_t *comm, ompi_group_t *group,
273273
int rc = OMPI_SUCCESS;
274274

275275
/* silence clang warning. newcomm should never be NULL */
276-
assert (NULL != newcomm);
276+
if (OPAL_UNLIKELY(NULL == newcomm)) {
277+
return OMPI_ERR_BAD_PARAM;
278+
}
277279

278280
lsize = group->grp_proc_count;
279281

@@ -462,7 +464,10 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key,
462464
}
463465

464466
/* silence clang warning. my_size should never be 0 here */
465-
assert (my_size > 0);
467+
if (OPAL_UNLIKELY(0 == my_size)) {
468+
rc = OMPI_ERR_BAD_PARAM;
469+
goto exit;
470+
}
466471

467472
sorted = (int *) calloc (my_size * 2, sizeof (int));
468473
if ( NULL == sorted) {
@@ -498,7 +503,7 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key,
498503
/* Step 2: determine all the information for the remote group */
499504
/* --------------------------------------------------------- */
500505
if ( inter ) {
501-
remote_group = MPI_GROUP_NULL;
506+
remote_group = &ompi_mpi_group_null.group;
502507
rsize = comm->c_remote_group->grp_proc_count;
503508
rresults = (int *) malloc ( rsize * 2 * sizeof(int));
504509
if ( NULL == rresults ) {
@@ -527,7 +532,7 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key,
527532
rc = OMPI_ERR_OUT_OF_RESOURCE;
528533
goto exit;
529534
}
530-
535+
531536
/* ok we can now fill this info */
532537
for( loc = 0, i = 0; i < rsize; i++ ) {
533538
if ( rresults[(2*i)+0] == color) {
@@ -536,7 +541,7 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key,
536541
loc++;
537542
}
538543
}
539-
544+
540545
/* the new array needs to be sorted so that it is in 'key' order */
541546
/* if two keys are equal then it is sorted in original rank order! */
542547
if (my_rsize > 1) {
@@ -697,7 +702,9 @@ ompi_comm_split_type(ompi_communicator_t *comm,
697702
ompi_comm_allgatherfct *allgatherfct=NULL;
698703

699704
/* silence clang warning. newcomm should never be NULL */
700-
assert (NULL != newcomm);
705+
if (OPAL_UNLIKELY(NULL == newcomm)) {
706+
return OMPI_ERR_BAD_PARAM;
707+
}
701708

702709
/* Step 1: determine all the information for the local group */
703710
/* --------------------------------------------------------- */
@@ -734,7 +741,10 @@ ompi_comm_split_type(ompi_communicator_t *comm,
734741
}
735742

736743
/* silence a clang warning about a 0-byte malloc. my_size can not be 0 here */
737-
assert (my_size > 0);
744+
if (OPAL_UNLIKELY(0 == my_size)) {
745+
rc = OMPI_ERR_BAD_PARAM;
746+
goto exit;
747+
}
738748

739749
sorted = (int *) malloc ( sizeof( int ) * my_size * 2);
740750
if ( NULL == sorted) {
@@ -1335,7 +1345,9 @@ static int ompi_comm_allgather_emulate_intra( void *inbuf, int incount,
13351345

13361346
/* silence clang warning about 0-byte malloc. neither of these values can
13371347
* be 0 here */
1338-
assert (rsize > 0 && outcount > 0);
1348+
if (OPAL_UNLIKELY(0 == rsize || 0 == outcount)) {
1349+
return OMPI_ERR_BAD_PARAM;
1350+
}
13391351

13401352
/* Step 1: the gather-step */
13411353
if ( 0 == rank ) {
@@ -1701,7 +1713,9 @@ int ompi_comm_determine_first ( ompi_communicator_t *intercomm, int high )
17011713
rsize= ompi_comm_remote_size (intercomm);
17021714

17031715
/* silence clang warnings. rsize can not be 0 here */
1704-
assert (rsize > 0);
1716+
if (OPAL_UNLIKELY(0 == rsize)) {
1717+
return OMPI_ERR_BAD_PARAM;
1718+
}
17051719

17061720
rdisps = (int *) calloc ( rsize, sizeof(int));
17071721
if ( NULL == rdisps ){

0 commit comments

Comments
 (0)