George, In topo_base_create.c at line 171, ompi_comm_enable ends up calling ompi_comm_activate which assigns MPI_COMM_NULL to new_comm when an error occurs ( see bail_on_error label )
So unless the correct fix is to change the behavior of ompi_comm_activate, I think the change is correct. Makes sense ? Cheers, Gilles On Tuesday, March 10, 2015, George Bosilca <[email protected]> wrote: > Gilles, > > I might misread these commit, but the changes proposed here do not look > correct to me. At no moment the new_comm can be equal to MPI_COMM_NULL in > this code (especially not at line 172 in the too_base_cart_create.c). > > George. > > > On Mar 9, 2015, at 02:26 , [email protected] <javascript:;> wrote: > > > > This is an automated email from the git hooks/post-receive script. It was > > generated because a ref change was pushed to the repository containing > > the project "open-mpi/ompi". > > > > The branch, master has been updated > > via 9107bf50776d540f50c29a1e263d5d40f16fe806 (commit) > > from a9044945fe4cedbcacd3415a2136aea470dade43 (commit) > > > > Those revisions listed above that are new to this repository have > > not appeared on any other notification email; so we list those > > revisions in full, below. > > > > - Log ----------------------------------------------------------------- > > > https://github.com/open-mpi/ompi/commit/9107bf50776d540f50c29a1e263d5d40f16fe806 > > > > commit 9107bf50776d540f50c29a1e263d5d40f16fe806 > > Author: Gilles Gouaillardet <[email protected] > <javascript:;>> > > Date: Mon Mar 9 15:22:22 2015 +0900 > > > > ompi/topo: fix misc errors > > as reported by Coverity with CIDs 1041232, 1041234, 1041235 > > 1269789 and 1269996 > > > > diff --git a/ompi/mca/topo/base/topo_base_cart_create.c > b/ompi/mca/topo/base/topo_base_cart_create.c > > index 6a678da..6d1c732 100644 > > --- a/ompi/mca/topo/base/topo_base_cart_create.c > > +++ b/ompi/mca/topo/base/topo_base_cart_create.c > > @@ -14,7 +14,7 @@ > > * Copyright (c) 2012-2013 Inria. All rights reserved. > > * Copyright (c) 2014 Los Alamos National Security, LLC. All right > > * reserved. > > - * Copyright (c) 2014 Research Organization for Information Science > > + * Copyright (c) 2014-2015 Research Organization for Information Science > > * and Technology (RIST). All rights reserved. > > * $COPYRIGHT$ > > * > > @@ -91,7 +91,6 @@ int mca_topo_base_cart_create(mca_topo_base_module_t > *topo, > > > > cart = OBJ_NEW(mca_topo_base_comm_cart_2_2_0_t); > > if( NULL == cart ) { > > - ompi_comm_free(&new_comm); > > return OMPI_ERR_OUT_OF_RESOURCE; > > } > > cart->ndims = ndims; > > @@ -172,11 +171,13 @@ int > mca_topo_base_cart_create(mca_topo_base_module_t *topo, > > new_rank, num_procs, topo_procs); > > if (OMPI_SUCCESS != ret) { > > /* something wrong happened during setting the communicator */ > > - new_comm->c_topo = NULL; > > - new_comm->c_flags &= ~OMPI_COMM_CART; > > free(topo_procs); > > OBJ_RELEASE(cart); > > - ompi_comm_free (&new_comm); > > + if (MPI_COMM_NULL != new_comm) { > > + new_comm->c_topo = NULL; > > + new_comm->c_flags &= ~OMPI_COMM_CART; > > + ompi_comm_free (&new_comm); > > + } > > return ret; > > } > > > > diff --git a/ompi/mca/topo/base/topo_base_dist_graph_create.c > b/ompi/mca/topo/base/topo_base_dist_graph_create.c > > index 43adf1a..dc676e3 100644 > > --- a/ompi/mca/topo/base/topo_base_dist_graph_create.c > > +++ b/ompi/mca/topo/base/topo_base_dist_graph_create.c > > @@ -8,7 +8,7 @@ > > * reserved. > > * Copyright (c) 2011-2013 Inria. All rights reserved. > > * Copyright (c) 2011-2013 Université Bordeaux 1 > > - * Copyright (c) 2014 Research Organization for Information Science > > + * Copyright (c) 2014-2015 Research Organization for Information Science > > * and Technology (RIST). All rights reserved. > > */ > > > > @@ -347,12 +347,14 @@ int > mca_topo_base_dist_graph_create(mca_topo_base_module_t* module, > > if ( NULL != topo->outw ) { > > free(topo->outw); > > } > > + if (MPI_COMM_NULL != new_comm) { > > + new_comm->c_topo->mtc.dist_graph = NULL; > > + new_comm->c_topo = NULL; > > + new_comm->c_flags &= ~OMPI_COMM_DIST_GRAPH; > > + ompi_comm_free (&new_comm); > > + } > > free(topo); > > free(topo_procs); > > - new_comm->c_topo = NULL; > > - new_comm->c_flags &= ~OMPI_COMM_DIST_GRAPH; > > - new_comm->c_topo->mtc.dist_graph = NULL; > > - ompi_comm_free (&new_comm); > > return ret; > > } > > *newcomm = new_comm; > > diff --git a/ompi/mca/topo/base/topo_base_graph_create.c > b/ompi/mca/topo/base/topo_base_graph_create.c > > index 990ac72..77a1be6 100644 > > --- a/ompi/mca/topo/base/topo_base_graph_create.c > > +++ b/ompi/mca/topo/base/topo_base_graph_create.c > > @@ -11,7 +11,7 @@ > > * All rights reserved. > > * Copyright (c) 2012-2013 Inria. All rights reserved. > > * Copyright (c) 2014 Cisco Systems, Inc. All rights reserved. > > - * Copyright (c) 2014 Research Organization for Information Science > > + * Copyright (c) 2014-2015 Research Organization for Information Science > > * and Technology (RIST). All rights reserved. > > * $COPYRIGHT$ > > * > > @@ -131,11 +131,13 @@ int > mca_topo_base_graph_create(mca_topo_base_module_t *topo, > > ret = ompi_comm_enable(old_comm, new_comm, > > new_rank, num_procs, topo_procs); > > if (OMPI_SUCCESS != ret) { > > - new_comm->c_topo = NULL; > > - new_comm->c_flags &= ~OMPI_COMM_GRAPH; > > free(topo_procs); > > OBJ_RELEASE(graph); > > - ompi_comm_free (&new_comm); > > + if (MPI_COMM_NULL != new_comm) { > > + new_comm->c_topo = NULL; > > + new_comm->c_flags &= ~OMPI_COMM_GRAPH; > > + ompi_comm_free (&new_comm); > > + } > > return ret; > > } > > > > > > > > ----------------------------------------------------------------------- > > > > Summary of changes: > > ompi/mca/topo/base/topo_base_cart_create.c | 11 ++++++----- > > ompi/mca/topo/base/topo_base_dist_graph_create.c | 12 +++++++----- > > ompi/mca/topo/base/topo_base_graph_create.c | 10 ++++++---- > > 3 files changed, 19 insertions(+), 14 deletions(-) > > > > > > hooks/post-receive > > -- > > open-mpi/ompi > > _______________________________________________ > > ompi-commits mailing list > > [email protected] <javascript:;> > > http://www.open-mpi.org/mailman/listinfo.cgi/ompi-commits > > _______________________________________________ > devel mailing list > [email protected] <javascript:;> > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel > Link to this post: > http://www.open-mpi.org/community/lists/devel/2015/03/17119.php
