Thread: Some confusion with realloc

I have a 2D array that I have inside a struct as follows:

Code:

struct Histostruct
{
    double **histogram;
    uint64_t numbins;
};
typedef struct Histostruct histostruct;

The struct is basically just so I can keep track of the size of the array, which will be numbins*3

I declare one here:

Code:

histostruct *histogram;
    if ((histogram=malloc(sizeof(histostruct)))==NULL)
    {
        printf("cannot allocate histogram structure\n");
        abort();
    }
    histogram->histogram = NULL;
    histogram->numbins = 0;

Now, in another function, I need to realloc the array every now and again. I do that as follows, where numbins is an integer which is calculated just prior to that:

Code:

if (histogram->histogram == NULL || histogram->numbins < numbins)
    {
        if ((histogram->histogram = realloc(histogram->histogram,numbins*sizeof(double *)))==NULL)
        {
            printf("Cannot allocate level 1\n");
            abort();
        }
        else
        {
            histogram->numbins = numbins;
        }
        for (i=0; i<numbins; i++)
        {
            if ((histogram->histogram[i] = realloc(histogram->histogram[i], 3*sizeof(double)))==NULL)
            {
                printf("Cannot allocate level 2 at i=%" PRIu64 "\n",i);
                abort();
            }
        }
    }

However, it crashes on the level 2 allocation. Is something wrong in the above, or is the problem elsewhere?

EDIT: it seems to have something to do with NULL pointers - if I don't declare histogram->histogram=NULL in the initial declaration, it fails on the level one allocation. Do I need to declare histogram->histogram[i]=NULL for all i before I can realloc the second level?

EDIT 2: turns out I don't need to call realloc for level 2, malloc does just fine. So while the issue is solved, I would still like to know why the second level realloc failed in the code above. Does the first argument of realloc have to be NULL?

Ah, that would be it. It had not been previous malloc'd. Neither had level 1, but it had been declared NULL explicitly so it worked.

It occurs to me that there is probably a memory leak in the above as well: because I am reallocing the first level without freeing the second level, the second level would be leaking memory. Can you confirm?

I modified it as follows:

Code:

if (histogram->histogram == NULL || histogram->numbins < numbins)
    {
        for (i=0; i<histogram->numbins; i++)
        {
            free(histogram->histogram[i]);
        }
        if ((histogram->histogram = realloc(histogram->histogram,numbins*sizeof(double *)))==NULL)
        {
            printf("Cannot allocate level 1\n");
            abort();
        }
        else
        {
            histogram->numbins = numbins;
        }
        for (i=0; i<numbins; i++)
        {
            if ((histogram->histogram[i] = calloc(3,sizeof(double)))==NULL)
            {
                printf("Cannot allocate level 2 at i=%" PRIu64 "\n",i);
                abort();
            }
        }
    }

Right you are, the memory should be preserved. So it looks like this would be the better way to do it:

Code:

    if (histogram->histogram == NULL || histogram->numbins < numbins)
    {
        if ((histogram->histogram = realloc(histogram->histogram,numbins*sizeof(double *)))==NULL)
        {
            printf("Cannot allocate level 1\n");
            abort();
        }
        for (i=histogram->numbins; i<numbins; i++)
        {
            if ((histogram->histogram[i] = calloc(3,sizeof(double)))==NULL)
            {
                printf("Cannot allocate level 2\n");
                abort();
            }
        }
        histogram->numbins = numbins;
    }