[petsc-dev] Allocating arrays

[Barry Smith]

   We currently have PetscMalloc(), PetscNew(), PetscNewLog(), PetscLogObjectMemory(), PetscFree(), PetscMalloc2-7(), PetscFree2-7()

    Now you want to introduce PetscMallocA(), PetscMallocA2-7(), PetscCalloc(), and PetscCallocA()

   I see three reasons for the suggested change

     1) insure that the space allocated is correct for the type of the variable,   great

     2) remove duplicate information about the type of the variable (that may not match),  ok

     3) Use an exceedingly ugly A is to prevent clashes with current code and allow backward compatibility with all current PETSc code, who cares

   I propose the following alternative:

      Remove the type arguments from PetscMalloc2-7()

      Add PetscMalloc1() 

      Add PetscCalloc1() 

      Use PetscCalloc1() to implement PetscNew() 

     This will

       1) insure the space allocated is correct for the type of the variable

       2) remove duplicate information about the variable type
  
       3) generate errors from cpp if the old/new PetscMalloc2-7() is used with the new/old headers preventing buggy code. Since few users use PetscMalloc2-7() I think this lack of backward capability is fine.

        4) have consistent naming with current code.

    I have no problem going through the grunt work of updating the PetscMalloc2-7(). When an API is wrong it should be fixed! (Not kept along with a new API). Insert amusing joke about Trilinos here.


    Barry








On Nov 30, 2013, at 12:50 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:

> I'm tired of mistakes matching types when allocating memory.
> 
>  PetscMPIInt *array;
> 
>  PetscMalloc(n*sizeof(PetscInt),&array);
> 
> This is not caught in debug mode due to:
> 
>  #if defined(PETSC_USE_DEBUG)
>  #define PetscMalloc2(m1,t1,r1,m2,t2,r2) (PetscMalloc((m1)*sizeof(t1),r1) || PetscMalloc((m2)*sizeof(t2),r2))
>  #else
>  #define PetscMalloc2(m1,t1,r1,m2,t2,r2) ((*(r2) = 0,PetscMalloc((m1)*sizeof(t1)+(m2)*sizeof(t2)+(PETSC_MEMALIGN-1),r1)) || (*(r2) = (t2*)PetscAddrAlign(*(r1)+m1),0))
>  #endif
> 
> At least it generates a warning when compiling optimized mode with
> 64-bit ints.  A similar problem arises when confusing PetscReal for
> PetscScalar.
> 
> But this can be much simpler:
> 
>  #define PetscMallocA(n,p) (PetscMalloc((n)*sizeof(**(p)),(p)))
> 
> Now it is always correct to write
> 
>  PetscMallocA(n,&array);
> 
> To break up the common sequence of PetscMalloc followed by PetscMemzero,
> I would add
> 
>  #define PetscCalloc(n,p) (PetscMalloc((n),(p)) || PetscMemzero(*(p),(n)))  /* Could use calloc() if we add support */
>  #define PetscCallocA(n,p) (PetscCalloc((n)*sizeof(**(p)),(p)))
> 
> And we can extend to more cases, e.g.,
> 
>  #if defined(PETSC_USE_DEBUG)
>  # define PetscMallocA2(n0,p0,n1,p1) (PetscMallocA((n0),(p0)) || PetscMallocA((n1),(p1)))
>  #else
>  # define PetscMallocA2(n0,p0,n1,p1) (PetscMalloc((n0)*sizeof(**(p0))+(n1)*sizeof(**(p1))+(PETSC_MEMALIGN-1),(p0)) || (*(void**)(p1) = PetscAddrAlign(*(p0)+(n0)),0))
>  #endif
> 
> This is used like
> 
>  PetscMallocA2(n0,&array0,n1,&array1);
> 
> 
> I've used similar macros in non-PETSc projects for years and I find it's
> a lot cleaner, less error-prone, and easier to change types.  Is anyone
> opposed to me adding PetscMallocA* functions?