wolfPack88
wolfPack88

Reputation: 4203

C freeing pointer to pointer

In my code, I have several pointers to pointers (e.g. float ** variables), and I seem to be having issues freeing up their memory so as to not cause memory leaks. Here is the code that I have written:

float *one, **part1, **part2;

one = malloc(sizeof(&one) * nx * nx);
part1 = malloc(sizeof(&part1) * nx);

if(one == NULL || part1 == NULL) {
 printf("Memory error.\n");
 exit(2);
}

for(k = 0; k < nx; k++)
 part1[k] = &one[k * nx];

one = malloc(sizeof(&one) * nx * nx);
part2 = malloc(sizeof(&part2) * nx);

if(one == NULL || part2 == NULL) {
 printf("Memory error.\n");
 exit(2);
}

for(k = 0; k < nx; k++)
 part2[k] = &one[k * nx];

... (Other code here)

for(k = 0; k < nx; k++) {
 free(part1[k]);
 free(part2[k]);
}

free(one);
free(part1);
free(part2);

This code runs through, does the calculations correctly, but then errors out in the free loop. It works for k = 0, but then when trying to free part1[1] and part2[1], it gives me the "glibc detected" error.

Upvotes: 0

Views: 2588

Answers (3)

wnoise
wnoise

Reputation: 9922

First of all:

one = malloc(sizeof(&one) * nx * nx);
part1 = malloc(sizeof(&part1) * nx);

These should be

one = malloc(sizeof(*one) * nx * nx);
part1 = malloc(sizeof(*part1) * nx);

You want to allocate a bunch of floats and float *s, not float **s and float ***s

Secondly, you make 4 allocations -- you allocate one, and indices into it in part1, and allocate one again, (forgetting the old address), and indices into it in part2.

This means you should have 4 free()s: part1, part2, and both chunks of memory to which one pointed. Because you overwrite the first one, you've lost that pointer and can't directly free() it. Fortunately you did save that pointer in part1[0], and can use that to free() all the memory pointed to.

Another (perhaps clearer and more idiomatic) option would be to allocate differently. Allocate part1, and then loop to allocate each part1[k], and the same for part2.

Upvotes: 2

casablanca
casablanca

Reputation: 70701

You have only 4 malloc calls, so you should only have 4 free calls. You never allocated anything in a loop, so why are you freeing in a loop?

part1[k] and part2[k] were never allocated by themselves, they just point to an area of the memory allocated to one, so you should just free one. Also, you have a memory leak here:

one = malloc(sizeof(&one) * nx * nx);
part1 = malloc(sizeof(&part1) * nx);

...

// *** You just lost the previous old pointer here *** //
one = malloc(sizeof(&one) * nx * nx);
part2 = malloc(sizeof(&part2) * nx);

The reason your code works for k = 0 is because part1[0] == &one[0] == one, i.e. part1[0] actually points to the beginning of the one block -- by freeing this, you are freeing the entire block.

And I'm not sure what your sizeof means. I'm guessing you wanted to allocate nx * nx floats -- if so, that should be sizeof(*one) -- *one is a float, but &one is the address of one, which is a float ** pointer.


You really want to be doing something like this:

one1 = malloc(sizeof(*one) * nx * nx);
part1 = malloc(sizeof(*part1) * nx);

...

for(k = 0; k < nx; k++)
 part1[k] = &one1[k * nx];

...

free(part1);
free(one1);

Upvotes: 1

James Crook
James Crook

Reputation: 1610

Look at how many malloc's you're doing and how many frees you are doing. The numbers don't match.

In general you cannot free a part of a malloc'd structure. You can only free it all. So at the end you want something more like:

... (Other code here)
/* four mallocs so four frees */
free(part1[0]);
free(part2[0]);
free(part1);
free(part2);

Upvotes: 1

Related Questions