0

I don't know why my unload function doesn't work.i wrote a recursive function to use in the unload function in dictionary.c and wrote the prototype in dictionary.h . i used a trie structure to be the dictionary and its root is a global pointer declared in dictionary.h. Valgrind shows me that the unload function only frees one node. here's my trie struct

typedef struct node

 {
                     bool is_word;
     struct node* children[27];

   }
 node;
node* root;

my recursive function and the unload function

/**
*free nodes of dictionary
*
*/
void free_memory(node* current_node)
{
    for(int i = 0 ; i < 27 ; i++)
    {
        if(current_node->children[i] == NULL)
        {
            free(current_node->children[i]);
        }
        else
        {
            current_node = current_node->children[i];
            free_memory(current_node);
        }
    }

}

/**
* Unloads dictionary from memory.  Returns true if successful else false.
*/
bool unload(void)
{    
   node* temp = root;
   free_memory(temp);
   if( temp == NULL)
   {
       return true;
    }
   else
   { 
       return false;
    }
}
  • Is root a global variable? I am afraid I did not understand you code. What about the main function? – user9640 Sep 20 '15 at 15:20
  • You should also free the current node when all of its children have been freed. – ChrisG Sep 20 '15 at 16:26
1

There are a few problems going on here. First, your test to see that it worked fails. After that, your free_memory function needs work.

Look at these two lines:

free_memory(temp);
if( temp == NULL)

When you actually free(temp) inside of free_memory(), it frees the memory, but it doesn't set temp to null. Temp will continue to point at something, even if it is garbage data. So, when you test with if( temp == NULL) it will probably never be NULL, even though the code has succeeded.

The free_memory() function needs work. For one thing, free(current_node->children[i]); doesn't do anything when current_node->children[i] is null, so that's dead code.

Next, look at this part:

    else
    {
        current_node = current_node->children[i];
        free_memory(current_node);
    }

The idea of the code is that you want to process the child. BUT, when you do this and return from the next lower free_memory() function call, you will be processing the child and not the rest of the parent. In other words, let's say that you were on the 2nd level of the trie when this section of code starts. It will switch to the 3rd level of the trie, do the call, return, and then continue forward processing the 3rd level nodes, skipping the remaining 2nd level nodes. The correct way to handle this is:

    else
    {
        free_memory(current_node->children[i]);
    }

Next, at the end of free_memory() and after freeing all the children, you still need to free the current node.

As another observation, I noted that you made this a function with a void return. This means that you never return any success or failure. Any particular reason? If you're having problems sorting that out, you might benefit from reading the following: Pset3 Binary Search problems

If this answers your question, please click on the check to accept. Let's keep up on forum housekeeping. ;-)

| improve this answer | |

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .