Not able to free memory from function

Question

I have a C program that implements trees. my cleanup function looks like this:

void cleanup_tree( TreeNode* root ){ printf("Called\n"); if(root->left!=NULL){ cleanup_tree(root->left); } if(root->right!= NULL){ cleanup_tree(root->right); } if(root->right==NULL &&root->left==NULL) { /*free(root);*/ free(root->word); free(root); root = NULL; } }

My Tree struct has

typedef struct TreeNode_st { char *word; // the word held in this node unsigned int frequency; // how many times it has been seen struct TreeNode_st *left; // node's left child struct TreeNode_st *right; // node's right child } TreeNode;

I am initialising a tree like this :

TreeNode* initTreeNode(){ TreeNode *mainNode= (TreeNode*)malloc(sizeof(TreeNode)); mainNode->frequency = 0 ; mainNode->word = NULL; mainNode->left = NULL; mainNode->right = NULL; return mainNode; }

in my main, I have called

TreeNode *mainNode =initTreeNode();

and I'm doing operations on it , and just before program exit, i called

cleanup_tree(mainNode);

Valgrind reported memory leaks, so just to test , i did I put
printf("~~~FINAL NULL TEST %s",mainNode->left->right->word); below my cleanup_tree line, And i'm able to see the word even now.

What am I doing wrong ?

Looks like if(root->right==NULL &&root->left==NULL) { only frees nodes with no children? — 001
– 001, Commented Oct 13, 2016 at 16:41
i am calling that function recursively, so It should to go the edges, set them free, so the parent will now be a node that has no children, and it will go up to the root — harveyslash
– harveyslash, Commented Oct 13, 2016 at 16:42
The variables don't set themselfs magically to NULL though. — tkausl
– tkausl, Commented Oct 13, 2016 at 16:43
That's a local variable, so not visible outside of your function. — tkausl
– tkausl, Commented Oct 13, 2016 at 16:45

Paul Ogilvie · Accepted Answer · 2016-10-13 16:45:28Z

There are two ways:

You pass it a pointer-to-a-pointer: void cleanup_tree( TreeNode **root)
You set the fields to NULL after the cleanup returns:

Currently, the changes made by the function are not reflected in the node parameter you passed.

Ad 2:

cleanup_tree(root->right); root->right= NULL;

dbush · Accepted Answer · 2016-10-13 16:45:35Z

You seem to be under the impression that setting root = NULL at the end of this function will be visible in the calling function so that the third if block gets called. That's not the case.

You want to always free() the word as well as the node itself.

void cleanup_tree( TreeNode* root ){ printf("Called\n"); if(root->left!=NULL){ cleanup_tree(root->left); } if(root->right!= NULL){ cleanup_tree(root->right); } free(root->word); free(root); }

when I removed my if (left node and right node are both null) condition , i get segmentation fault
@harvey_slash There must be some other issue, then. What does valgrind say?
@Paul Ogilvie's answer worked, if im setting left and right to NULL, valgrind is happy

Collectives™ on Stack Overflow

Not able to free memory from function

2 Answers 2

Comments

3 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Related