What does free all pointers in children mean? (Unload function in Pset 5)

Question

In the walkthrough video, it is told that we should free all pointers in children (trie). Does that mean that all pointers should be set to null?

My program passes CS50 and valgrind doesn't indicate any leaks. But, by inserting printf's inside the freenode function, it is seen that pointers(to trie nodes) are not reset to null. Thus, root(first trie node) is not set to null.

Earlier, I used root as a condition in unload() to return. If root was null, true would be returned, else false. This caused the program to print that it could not be unloaded.

So how do I reset the pointers to NULL, and, what should be the condition in unload for true or false to be returned?

unload();

bool unload(void)
{
freenode(root);
return true;
}

freenode():

// Frees the specified node
void freenode(node* ptr)
{
// Iterate through children array
for (int i = 0; i < 27; i++)
{
    if (ptr -> children[i] !=NULL)
    {
        freenode(ptr -> children[i]);
    }
    else
    {
        continue;
    }
}
free(ptr);

// Does not seem to work
ptr = NULL;

return;
}

Answer 1

Hmmm, one thing i could maybe think of is that the pointer inside the function is actually a copy of the root pointer (the address is the same, so it gets freed, but the actual pointer is a copy because everything gets passed by value in C, so it's a copy of the pointer).

So when you call free(ptr), it doesn't matter that ptr is a copy, because what is freed is the address.

But when you do ptr=null; here what you're setting to null is "ptr", which is a copy. The real node outside the function doesn't ever get changed.

When you initialize a pointer, it's going to be NULL if you initialize it to NULL. If you do something like: "node* root;", it can be null or it can be anything. It can have a garbage value just like anything else.

Setting a pointer to null should set it to null.

Your unload function can only return true. If you still want to make sure, you can try passing a pointer to a pointer to your recursive function. So your function prototype would be

void freenode(node** ptr);

and the function call would be:

freenode(&root);

You will also have to adjust the recursive calls and make sure you dereference when needed. Personally, I find it a bit ugly, very error prone, and unnecessary :)

As I said in the previous reply, the only way to make sure you are freeing all the memory is to run valgrind. If you try your unload function with both dictionaries and valgrind detects no leaks or errors, it means that it works.

Answer 2

Setting the pointer to NULL after freeing it is not really necessary and it may end up hiding some bugs in the implementation. Still, I really can't think of any reason why it wouldn't set it to NULL in your code the way you did it. It should work The fact is, you could still be freeing the memory wrong and root could still be NULL, so it doesn't really work as a check. The only thing that works for making sure the memory has been freed and that there are no errors is valgrind.

BTW, the else continue; part of the code is not necessary. It will continue anyway :)