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/


Reply via email to