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?

If the pointer is not NULL and has not been previously allocted memory by malloc() you will have problems. Remember by default, unless global, or static, or been assigned a NULL value a pointer will not be NULL, and realloc() requires either a NULL pointer or a pointer that has already been allocated by malloc().

Jim

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();
            }
        }
    }

---

If your size is increasing the memory should remain intact, the newly allocated memory will be undetermined.

If you free() the memory for the inner pointers then you will loose any information that those pointers point to, is that really what you want? If so then you would probably be better off just using free() followed by malloc()/calloc() instead of realloc().

Jim

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;
    }

---