Safe allocation of a structure containing various arrays in c

Question

I have something like this in my code

typedef struct ts_fem_mesh
{   
    double          **vertices;
    unsigned int    **triangles;
    unsigned int    n_ver;
    unsigned int    n_tri;
} fem_mesh;

fem_mesh *fem_mesh_new(unsigned int n_ver, unsigned int n_tri)
{
    fem_mesh *mesh;
    mesh = (fem_mesh *)malloc(sizeof(fem_mesh));

    mesh->n_ver = n_ver;
    mesh->n_tri = n_tri;

    mesh->vertices = (double **)calloc(n_ver, sizeof(double *));
    mesh->triangles = (unsigned int **)calloc(n_tri, sizeof(unsigned int *));

    int i;
    for(i=0;i<n_ver;i++)
        mesh->vertices[i] = (double *)calloc(2, sizeof(double));
    for(i=0;i<n_tri;i++)
        mesh->triangles[i] = (unsigned int *)calloc(3, sizeof(unsigned int));
    return mesh;

}

Usually, when I call fem_mesh_new, I use very large number for n_ver and n_tri, which sometimes leads to allocation error (not enough space).

Even if I get this kind of error, my program should advice user and follow in execution. In this case I want to free all the thing I have allocated since the error point (i.e. I get error when I try to allocate mesh->triangles, but mesh->vertices is allocated, so I want to free mesh->vertices)

Is there a simpler way to do that? The only way I can think (which is the one I'd like to avoid) is to full my code of many if (x==NULL), but this is very annoying because of the order in which memory is allocated (in every point where I could get an error, I should have to write code that free all things allocated since that point).

Don't know if it's clear, hope some one could bring some help :)

score 2 · Answer 1 · answered Apr 01 '11 at 09:14

You always need to test the return value of malloc().

I think this is a good use of goto:

int foo(void) {
    a = malloc();
    if (!a) goto aerr;
    b = malloc();
    if (!b) goto berr;
    c = malloc();
    if (!c) goto cerr;
    d = malloc();
    if (!d) goto derr;

    /* ... use memory ... */

    free(d);
derr:
    free(c);
cerr:
    free(b);
berr:
    free(a);
aerr:
    return 0;
}

score 1 · Accepted Answer · answered Apr 01 '11 at 09:12

Since dynamic allocation can fail in each allocation, you should check if it's successful for each use of it even if it's annoying. But you can do it in 3 points only:

after mesh = (fem_mesh *)malloc(sizeof(fem_mesh));

after mesh->triangles = (unsigned int **)calloc(n_tri, sizeof(unsigned int *)); and check for both allocations.

after your loops, and check in loop.

Safe allocation of a structure containing various arrays in c

2 Answers2