That looks reasonable to me, but I'd also re-indent the body of the else{} (i.e., remove 4 spaces from each).
George? On Apr 14, 2011, at 10:48 AM, Pascal Deveze wrote: > Calling MPI_Type_create_hindexed(int count, int array_of_blocklengths[], > MPI_Aint array_of_displacements[], MPI_Datatype oldtype, > MPI_Datatype *newtype) > with a count parameter of 1 causes a loss of memory detected by valgrind : > > ==2053== 576 (448 direct, 128 indirect) bytes in 1 blocks are definitely lost > in loss record 157 of 182 > ==2053== at 0x4C2415D: malloc (vg_replace_malloc.c:195) > ==2053== by 0x4E7CEC7: opal_obj_new (opal_object.h:469) > ==2053== by 0x4E7D134: ompi_datatype_create (ompi_datatype_create.c:71) > ==2053== by 0x4E7D58E: ompi_datatype_create_hindexed > (ompi_datatype_create_indexed.c:89) > ==2053== by 0x4EA74D0: PMPI_Type_create_hindexed > (ptype_create_hindexed.c:75) > ==2053== by 0x401A5C: main (in /home_nfs/xxx/type_create_hindexed) > > This can be reproduced with the following trivial code: > ===================================== > #include "mpi.h" > > MPI_Datatype newtype; > int lg[3]; > MPI_Aint disp[3]; > > int main(int argc, char **argv) { > MPI_Init(&argc,&argv); > > disp[0] = (MPI_Aint)disp; > disp[1] = (MPI_Aint)disp+1; > lg[0] = 5; > lg[1] = 5; > > MPI_Type_create_hindexed(1, lg, disp, MPI_BYTE, &newtype); > MPI_Type_free(&newtype); > > MPI_Finalize(); > } > ====================================== > If MPI_Type_create_hindexed() is called with a count parameter greater 1, > valgrind does not detect any lost record. > > Patch proposed: > > hg diff ompi/datatype/ompi_datatype_create_indexed.c > diff -r a2d94a70f474 ompi/datatype/ompi_datatype_create_indexed.c > --- a/ompi/datatype/ompi_datatype_create_indexed.c Wed Mar 30 18:47:31 > 2011 +0200 > +++ b/ompi/datatype/ompi_datatype_create_indexed.c Thu Apr 14 16:16:08 > 2011 +0200 > @@ -91,11 +91,6 @@ > dLength = pBlockLength[0]; > endat = disp + dLength * extent; > - if( 1 >= count ) { > - pdt = ompi_datatype_create( oldType->super.desc.used + 2 ); > - /* multiply by count to make it zero if count is zero */ > - ompi_datatype_add( pdt, oldType, count * dLength, disp, extent ); > - } else { > for( i = 1; i < count; i++ ) { > if( endat == pDisp[i] ) { > /* contiguous with the previsious */ > @@ -109,7 +104,6 @@ > } > } > ompi_datatype_add( pdt, oldType, dLength, disp, extent ); > - } > *newType = pdt; > return OMPI_SUCCESS; > } > > Explanation: > The case (0 == count) was resolved before by returning. > The problem is that, in the case ( 1 >= count ), ompi_datatype_create() is > called again (it has been just called before). > In fact the case (1 == count) is not different of the case (1 < count), so > it is possible to just avoid the if-else statement. > > We need a patch for OpenMPI 1.5 branch. > > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/