when to free an allocated memory in c?

Question

I tried using valgrind to run the following code. Some heap leak is observed in the functions below. My question is where is the correct position to free the memory being allocated? And also in terms of recursion such as the trav = trav -> right, do i need to free it up also?

TTreeNode *makeNewNode(char *name, char *phoneNum)
{
   
    TTreeNode *newNode = (TTreeNode *)malloc(sizeof(TTreeNode));
    name = (char *)malloc(strlen(name)+1);
    newNode->name = name;
//phone num is a array pointer, do i need to alloc memory to it too?
    strcpy(newNode->phoneNum, phoneNum);
    newNode->left = NULL;
    newNode->right = NULL;
    return newNode;
}

void addPerson(char *name, char *phoneNum) {
    if(findPerson(name) == NULL) 
    {
        TTreeNode *node = makeNewNode(name, phoneNum);
        addNode(&_root, node);
    }
    else
        printf("%s is already in phonebook.\n", name);
}

void addNode(TTreeNode **root, TTreeNode *node)
{
    // printf("%s\n", *root);
    if (*root == NULL)
    {
        *root = makeNewNode(node->name, node->phoneNum);
        return;
    }

    // printf("%s\n", *root);

    TTreeNode *trav;
    trav = (TTreeNode *)malloc(sizeof(TTreeNode));
    trav = *root;
    while (trav != NULL)
    { 
       int cmp = strcmp(trav->name, node->name);
        if (cmp < 0)
        {
            if (trav->right == NULL)
            {
                trav->right = node;
                
                break;
            }
            else
               {trav = trav->right;
                }
        }

        else
        {
            if (trav->left == NULL)
            {
                trav->left = node;
                break;
                
            }
            else
                {trav = trav->left;
                }
        }
    }
  
}

this is how i free the memory:

void freenode(TTreeNode *node)
{
    // Frees the memory used by node.
    if (node != NULL)
    {
        free(node);
        
    }
    node = NULL;
}

the main function:

int main() {
    TData data[ITEMS] = {{"Fred Astaire", "95551234"}, {"Jean Valjean",
    "95558764"}, {"Gal Gadoti", "95551123"}, {"Aiken Dueet", "95558876"},
    {"Victor Hugo", "95524601"}};

    int i;

    for(i = 0; i<ITEMS; i++){
        printf("Adding %s, phone number %s\n", data[i].name, data[i].tel);
        addPerson(data[i].name, data[i].tel);
    }

    printf("\nNow retrieiving stored data.\n");
    char *result;

    for(i=0; i<ITEMS; i++) {
        printResult(data[i].name);
    }

printf("\nPrinting entire phonebook.\n");
    print_phonebook();

    printf("\nDeleting Aiken Dueet.\n");
    delPerson("Aiken Dueet");
    print_phonebook();

   
}

When to `free` is up to the code that uses these functions (which is code you have not shown). Write a function that frees the entire tree that the user of these APIs can call. — kaylum, Feb 03 '22 at 04:46
"*I tried using valgrind to run the following code*". This can't be the actual complete code you ran in valgrind because there is no `main`. Please provide a complete [mre] if you need further help. — kaylum, Feb 03 '22 at 04:47
Note: It looks like `makeNewNode` doesn't actually _use_ the `name` argument, except to find out how long it is and allocate space for a new string of the same length. — Chris, Feb 03 '22 at 04:55
"*this is how i free the memory*". But you never call that `freenode` function. — kaylum, Feb 03 '22 at 05:00
the name argument is used to copy the values inside to the newnode->name. — starry, Feb 03 '22 at 05:02
yes my question is when do i need to call the freenode function? — starry, Feb 03 '22 at 05:03
Whenever you don't need the tree anymore. Say right before the `main` ends. But that function only frees one node. You need to free the whole tree and should have a function to do that. — kaylum, Feb 03 '22 at 05:13

klutt · Answer 1 · 2022-02-03T06:57:35.073

In practice you need to free before reusing a pointer for something else. Not doing so is what typically cause memory leaks.

You don't need to free before exiting. Usually, the operating system will take care of that for you. There are rare exceptions though.

Other than that, a rule of thumb is to free the memory when you are done using it.

Your cleanup function is not good. Here is a proper one:

void freenode(TTreeNode *node)
{
    if (node != NULL)
    {
        freenode(node->left);
        freenode(node->right);

        free(node->phoneNum);
        free(node->name);
    }

    free(node);
}

Oh, and don't cast malloc.

Do I cast the result of malloc? And do I pass the size of the type or the object?

'It's also considered good practice to free everything before exiting too' no. On non-trivial OS with memory managers, it's bad practice. On many embedded systems that run forever, thete is nothing to exit to anyway. — Martin James, Feb 03 '22 at 05:48

score 0 · Answer 2 · answered Feb 03 '22 at 05:24

You don't have to free memory when iterating through linked list. If you want to free each node after you don't need to use the linked list any more, you can write a function which takes the linked list as a parameter and iterates through it by freeing each node until it reaches to NULL.

score 0 · Answer 3 · answered Feb 03 '22 at 09:05

It is really not necessary to free the memory you used especially through iterations in a list, whether it is linked or not. I understand you want to free every node after the list is useless for you, so you have to write a function that frees every node until node==NULL.

This answered question might be helpful for the function you need to create.

when to free an allocated memory in c?

3 Answers3