CODEWITHSUNDEEP
cmallocfreedynamic-memory-allocationvoid-pointers
MikeS159
MikeS159

Reputation: 1974

Freeing elements of a void pointer array

I am debugging some code written by someone else which has several memory leaks detected by Valgrind. Part of the code which I believe is causing the problems involves void pointers:

int noElements = 100;
void** ptr = malloc( noElements * sizeof(void*));
for(int i = 0; i < noElements; i++)
{
    ptr[i] = malloc(sizeof(void*));
}

//Operation filling the array and doing calculation

free(ptr); //This will not clear the underlying array causing the memory leak?

I created a small test program to check the error is where I thought it was (not freeing each ptr[i] before calling free(ptr);), but I am getting errors when trying to free elements. My test program looks like this:

int main()
{
    int num = 10000000; //Large array so I can clearly see memory usage
    void** ptr = (void**) malloc(sizeof(void*) * num);

    for (int i = 0; i < num; i++)
    {
        ptr[i] = malloc(sizeof(void*));
        ptr[i] = &i;    //If I never point elements at anything, this code works
    }

    for (int i = 0; i < num; i++)
    {       
        free(ptr[i]);       //Crashes here if the elements have been pointed at a variable
    }
    free(ptr);  

    return 0;
}

Why would calling free on each array element cause the program to crash if the pointers have actually been assigned?

Upvotes: 0

Views: 1118

Answers (2)

Sourav Ghosh
Sourav Ghosh

Reputation: 134396

In your code

  ptr[i] = &i;

creates three issues.

  1. It makes the actual returned pointer by malloc() to be lost (you're overwriting it), so you have no shot at free()-ing it later, leading to memory leak.
  2. i is a local scoped variable (the scope of for loop body), and you're storing the address of the variable to be used outside the scope (i.e., after the lifetime is over). Outside the scope attempt to access the address of it will invoke undefined behavior.
  3. (Ignoring point 2) Attempt to free() the ptr[i] will cause undefined behavior again, as the pointer is not returned by memory allocator function.

Upvotes: 3

Gerhardh
Gerhardh

Reputation: 12404

Your pointers have been assigned a value which is not what you want.

for (int i = 0; i < num; i++)
{
    ptr[i] = malloc(sizeof(void*));
    ptr[i] = &i;    //If I never point elements at anything, this code works
}

First you assign a valid memory address that is returned by malloc(). This address could be free'd using free(). But then you assign another address. The address of your local variable i. This memory location was not allocated using malloc() and hence cannot be free'd. By assigning this address you've lost the dynamically allocated address causing a memory leak.

Your comment is wrong. You already assigned a value that ptr[i] points to some valid address. You don't need to assign again. And you should not.

Upvotes: 1

Related Questions