free up memory on a double pointer

Question

//char char **p; declared in .h file
size_t bs = 5;
size_t Size = sizeof(obj);
p = (char**)malloc(bs);
for (size_t i = 0; i < bs;i++){p[i] = (char*)malloc(Size);}

for (size_t j = 0; j < bs-1; j ++){p[j] = &(p[j + 1][0]); }

for (size_t i = 0; i < bs; i++){free(p[i]);}

free(p);

my code stalls up when trying to free the last element of p in the for loop. Anyone what i might be doing wrong?

EDIT: I still have the same problem even when changing it to (char*)malloc(bs sizeof(char *));

this still does not work:

size_t bs = 5;
size_t Size = sizeof(obj);
p = (char**)malloc(bs* sizeof(char *));
for (size_t i = 0; i < bs;i++){p[i] = (char*)malloc(Size);}
for (size_t j = 0; j < bs-1; j ++){p[j] = &(p[j + 1][0]); }
for (size_t i = 0; i < bs; i++){free(p[i]);}
free(p);

using new instead of malloc does not solve the issue either

However this code frees up the memory fine.

size_t bs = 5;
size_t Size = sizeof(obj);
p = (char**)malloc(bs* sizeof(char *));
for (size_t i = 0; i < bs;i++){p[i] = (char*)malloc(Size);}
for (size_t i = 0; i < bs; i++){free(p[i]);}
free(p);

so the problem seem to be something with this piece of code

for(size_t j = 0; j < bs-1; j ++){p[j] = &(p[j + 1][0]); }

I want this to be an implicit linked list, anyone know have an idea what i am doing wrong?

Is it C or C++? I guess this is valid in either, but maybe pick just one. — zero298, Jan 24 '14 at 21:19
possible duplicate of [free a double pointer](http://stackoverflow.com/questions/11015360/free-a-double-pointer) — zero298, Jan 24 '14 at 21:20
You realize that `&(pool[j + 1][0])` is exactly the same as `pool[j + 1]` ? — Ben Voigt, Jan 24 '14 at 21:31
@Ed: The iteration index is declared inside the `for` parentheses; that looks like a C++ feature to me. (did it get added in later C versions? — Ben Voigt, Jan 24 '14 at 21:32
In C: do not cast the result of `malloc`. But in C++ you need to do so. — pzaenger, Jan 24 '14 at 21:38
@BenVoigt - To my knowledge it has been a part of C for quite a long time — Ed Heal, Jan 24 '14 at 21:39
@EdHeal: Yes, I know. But the OP has used `malloc`, therefore I wrote this :) — pzaenger, Jan 24 '14 at 21:39
@Ed: In C++, you do use `malloc` if you are passing something to an API that requires malloc'd data (because it will use `free`). C++ programmers ought to know basic usage of `std::malloc` — Ben Voigt, Jan 24 '14 at 21:40
@BenVoigt - This is a very raw occasion these days. Anyway this is not implied in the OP. So one assumes this is not C++ — Ed Heal, Jan 24 '14 at 21:42

score 2 · Answer 1 · answered Jan 24 '14 at 21:20

2

You are not allocating enough space for the pointers. Change to

p = malloc(bs * sizeof(char*));

answered Jan 24 '14 at 21:20

cup

7,589
4
19
42

This is one reason why C++ is better. It would have allocated the proper amount with `new`. – Mark Ransom Jan 24 '14 at 21:29
1

@Mark: I guess you meant to say "why C++'s `new[]` operator is better"? `malloc` works exactly the same in C++, and C++ allocation functions such as `::operator new()` also require the number of bytes. – Ben Voigt Jan 24 '14 at 21:33

deviantfan · Answer 2 · 2014-01-24T21:43:08.177

1

At the first malloc, you don´t need 5 byte, but 5 times a pointer.

p = (char**)malloc(bs * sizeof(char *));

edited Jan 24 '14 at 21:43

answered Jan 24 '14 at 21:20

deviantfan

11,268
3
32
49

okay i changed it to (char**)malloc(bs * sizeof(char *)); its still giving me the same problem. – user3233447 Jan 24 '14 at 21:30
1

This is true, but *leak* isn't the right word for it. It's actually an overflow, because elements past the end are accessed. – Ben Voigt Jan 24 '14 at 21:30
1

Technically a leak is when you forget to free something. You've identified the problem but given it the wrong name. – Mark Ransom Jan 24 '14 at 21:30

Chris Dodd · Answer 3 · 2014-01-24T23:16:18.427

The problem is that you:

allocate an array of 5 pointers
allocate 5 arrays of characters and store them in that first array
move those pointers down in the array, overwriting (and losing) the first pointer and duplicating the last one
attempt to free the 5 pointers in the array.

So on this last step, you free a pointer twice (as the last two entries at p[3] and p[4] are the same), causing undefined behavior.

You say you want "an implicit linked list", implying that you're trying to stuff pointers into the objects (rather than into the top level array, as you are doing), in which case you want something like:

for(size_t j = 0; j < bs-1; j ++) { *(char **)p[j] = p[j + 1]); }
*(char **)p[bs-1] = 0;  // null terminate the linked list

this assumes that obj is defined something like:

struct obj {
    struct obj *next;
    // more fields

free up memory on a double pointer

3 Answers3