0

I have a question... If I have a Binarytree like this:

typedef struct
{
   char* text;
   char* line;
   struct Node* left;
   struct Node* right;
}Node

and have a start function:

Node *start(Node* ptr)
{
if(ptr == NULL)
{
 ptr = (Node*)malloc(sizeof(Node));
    ptr->text = "some text";
    ptr->line = "another text";
    ptr->left = NULL;
    ptr->right = NULL;
}
return (ptr);
}

And a funtion to add left child in binarytree, if root is not empty and here I must give my text and line dynamic memory.

  Node* addNode(Node* ptr)
  {
    if(ptr != NULL)
    {
    Node* leftNode = (Node*)malloc(sizeof(zeiger));
    char* text = (char*) malloc(100*sizeof(char));
    char* line = (char*) malloc(100*sizeof(char));

    line = "some text 2";
    text = "another text";
    if(leftNode !=NULL)
    {
        leftNode->text = text;
        leftNode->line = line;
        leftNode->right = NULL;
        ptr->left = leftNode;
    }
    return leftNode;
}
return ptr;

}

Now the problem ist I want to free everything in tree so I have a a function like this to call himself if left or right or root is not NULL. So i saw some codes that I should free the root and not the datas.

void freeTree(Node* root)
{
  while(1)
  {
   if(root == NULL)
  {
  break;
}
else if (root->left != NULL)
{
  freeTree(root->left);
  root->left = NULL;
}
else if (root->right) {
        freeTree(root->right);
  root->right = NULL;
}
else
  {
  free(root);
  }
 }
}

Main.c

int main(int argc, char const *argv[]) {

Node* root = NULL;
root = start(root);
root = addNode(root);
freeTree(root);
return 0;
}
Newuser1234567
  • 23
  • 6
  • 1
    You have to free everything, all nodes. If you allocate the memory yourself, you have to free it yourself or it's a memory leak, especially if you only free the root node, the children are not freed ever by itself by some magic. – xander Dec 05 '17 at 10:15
  • 2
    `char* line = (char*) malloc(100*sizeof(char));` `line = "some text 2";` Is a memory leak. As you immediately lose the pointer to the allocated memory, you'll never be able to ever free it. – Lanting Dec 05 '17 at 10:30
  • @Lanting should i free this after it parsed to tree? – Newuser1234567 Dec 05 '17 at 10:50
  • It's about ownership, if the node 'owns' the line of text, then every time the pointer gets reassigned, the old value should be freed. Your node, however, does not own the string constant. You cannot call free on a pointer to a string constant. If you want your node to "own" the string, you should `strncpy` it to the buffer you just allocated. https://stackoverflow.com/questions/10063222/freeing-strings-in-c – Lanting Dec 05 '17 at 10:56

1 Answers1

2

You shouldn't free memory that you didn't malloc.

Here the first node's pointers are pointing to a string literal. You shouldn't call free() on them.

You should allocate memory in the initial node also and then try to copy that string literal. Now every node is similar.

Now you free the memory of each node. Which node to do first? The post order traversal would be the one. First the children, then the parent will should be freed, otherwise you will have memory leak.

void freeTree(Node *root){
   if( root ){
     freeTree(root->left);
     freeTree(root->right);
     free(root->text); // root->text was allocated calling malloc
     free(root->line); // same for root->line
     free(root);
   }
}

root->text was allocated calling malloc - You have to allocate dynamically to be able to call free on it. In your code the initial node's text is pointing to a string literal. You need to do it proper way as mentioned in answer - (allocate - and copy the string into newly allocated memory).

user2736738
  • 30,591
  • 5
  • 42
  • 56