Free memory allocated in a different function?

Question

I'm trying to learn C and I'm currently trying to write a basic stack data structure, but I can't seem to get basic malloc/free right.

Here's the code I've been using (I'm just posting a small part here to illustrate a specific problem, not the total code, but the error message was generated just by running this example code in valgrind)

#include <stdio.h>
#include <stdlib.h>

typedef struct Entry {
    struct Entry *previous;
    int value;
} Entry;

void destroyEntry(Entry entry);

int main(int argc, char *argv[])
{
    Entry* apple;
    apple = malloc(sizeof(Entry));
    destroyEntry(*(apple));
    return 0;
}

void destroyEntry(Entry entry)
{
    Entry *entry_ptr = &entry;
    free(entry_ptr);
    return;
}

When I run it through valgrind with --leak-check=full --track-origins=yes, I get the following error:

==20674== Invalid free() / delete / delete[] / realloc()
==20674==    at 0x4028E58: free (vg_replace_malloc.c:427)
==20674==    by 0x80485B2: destroyEntry (testing.c:53)
==20674==    by 0x8048477: main (testing.c:26)
==20674==  Address 0xbecc0070 is on thread 1's stack

I think this error means that the destroyEntry function is not allowed to modify memory allocated explicitly in main. Is that right? Why can't I just free the memory I allocated in main in another function? (and is this behavior somehow specific to main?)

Solution 1

Whenever you pass a parameter to a function, a copy is made, and the function works on that copy. So in your case, you are trying to free a copy of the original object, which doesn't make any sense.

You should modify your function to take a pointer, and then you can have it call free directly on that pointer.

Solution 2

This is passing by value, which means that copy is created, thus you try to free the memory, where local variable entry resides. Note that entry is an object with automatic storage duration and memory where it resides will be freed automatically when your program goes out of scope of destroyEntry function.

void destroyEntry(Entry entry)
{
    Entry *entry_ptr = &entry;
    free(entry_ptr);
    return;
}

Your function should take a pointer (passing by reference):

void destroyEntry(Entry *entry)
{
    free(entry);
}

Then instead of destroyEntry(*(apple)); you just call destroyEntry(apple);. Note that if there is no other functionality connected with destroyEntry function, it's redundant and it's better to just directly call free(apple).

Solution 3

The other answers here point out the main problem -- because you dereference your apple when you call destroyEntry in main(), it passes by reference, creating a copy.

Even once you know your problem, it helps to go back to the error and try to connect the text of what you're seeing to the problem, so that the next time it comes up you might be more likely to figure it out quickly. I find C and C++ errors can seem maddeningly ambiguous sometimes.

Generally, when I'm having trouble freeing pointers or deleting objects, I like to print out addresses, especially right when I allocate it and right when I try to free it. valgrind already gave you the address of the bad pointer, but it helps to compare it to a good one.

int main()
{
  Entry * apple;
  apple = malloc(sizeof(Entry));
  printf("apple's address = %p", apple);  // Prints the address of 'apple'
  free(apple);   // You know this will work
}

After doing that, you'd notice the printf() statement gave you an address something like 0x8024712 (just making up an address in the right general range), but your valgrind output gave 0x4028E58. You'd notice they're in two very different places (in fact, "0x4..." is on the stack, not the heap where malloc() allocates from, but I'm assuming if you're just starting out that's not a red flag for you yet), so you know you're trying to free memory from the wrong place, hence "invalid free()".

So from there you can say to yourself "Okay, somehow my pointer is getting corrupted." You already boiled down your problem to a small, compilable example, so it won't take you long to solve it from there.

TL;DR - when running into pointer-related errors, try printing the addresses or finding them in your favorite debugger. It often at least points you in the right direction.

None of this is to discourage posting your question on Stack Exchange, of course. Hundreds of programmers will likely benefit from your having done so.

Comments

[Jeff Tratner]

Thank you for posting this...it's a really nice point about figuring out errors with pointers and such. I've been learning about stack vs. heap, but didn't realize that they had generalized memory addresses. (so, NULL --> 0x0, stack-->0x4 and heap is anything else?).

[Jeff Tratner]

Also, I can't make a one character change to your post, but in the printf string: "apple's address = %d" the format string should really be %p, not %d, right?

[gkimsey]

I can't tell you with 100% certainty that heap addresses will always be '0x8...' and stack addresses will always be '0x4...'. To be honest, I do mostly embedded programming these days where the address space is very well defined and your process is the only one running. However, my understanding is that on x86, each process gets its own virtual address space and in my experience those "rules" seem to hold true. Thanks for pointing out the %d problem. It works (in that it does print the address), but it's harder to read. I usually use "0x%x" myself as that's how I'm used to seeing it.

[user124384]

So just to be clear, passing *(apple) into the DestroyEntry function creates a new Entry struct that's separate from the original apple one?