0

My unload function is leaking memory. Valgrind says that 8,960 bytes in 40 blocks were definitely lost, and 38,752 bytes in 173 blocks were indirectly lost. I'm not sure why...my unload function makes perfect sense to me. I would greatly appreciate it if someone pointed out an error in my code. Here's the code:

void recursive_unload(node* trav)
{
    for (int i = 0; i < 27; i++)
    {
        if (trav -> children[i] != NULL)
            recursive_unload(trav -> children[i]);
    }
    free(trav);
}

bool unload(void)
{
    node* trav = head;
    recursive_unload(trav);
    return true;
}

Valgrind also points at a specific line in my load function for some reason. I don't see any problem with it, but I will post it as well, just in case my error was here:

bool load(const char *dictionary)
{
    FILE* dic = fopen(dictionary, "r");
    unsigned int num_words = 0;
    pnum_words = &num_words;

    if (dic == NULL)
        return false;

    head = NULL;
    head = malloc(sizeof(node));
    if (head == NULL)
        exit(0);
    head -> is_word = false;
    for (int i = 0; i < 27; i++)
        head -> children[i] = NULL;

    node* trav = head;

    for (int c = fgetc(dic); c != EOF; c = fgetc(dic))
    {

        if (c == '\n')
        {
            trav -> is_word = true;
            trav = head;
            num_words++;
        }
        else if (((c == '\'') && (trav -> children[26] == NULL)) || (trav -> children[c - 'a'] == NULL))
        {    
            if (c == '\'')
            {    
                trav -> children[26] = malloc(sizeof(node)); //THIS LINE IS POINTED TO BY VALGRIND//
                trav = trav -> children[26];
            }
            else
            {    
                trav -> children[c - 'a'] = malloc(sizeof(node));
                trav = trav -> children[c - 'a'];
            }
        }
        else 
        {   
            if (c == '\'')
                trav = trav -> children[26];
            else 
                trav = trav -> children[c - 'a'];
        }
    }
    fclose(dic);
    return true;
}

Thanks again for any help.

1

You need a second check in that large condition

((c == '\'') && (trav -> children[26] == NULL) || (c != '\'') && (trav -> children[c - 'a'] == NULL))

as it might happen that c == '\'' and trav -> children[26] != NULL, but trav -> children[c - 'a'] == NULL, making you replace an existing non-NULL trav -> children[26].

It would be easier if you calculated an index first, that way you had the special treatment of '\'' in one place.

Also, you initialise the root node (setting is_word to false and all 27 children[i] to NULL), but you don't do the same for the child nodes you malloc in between. You could use calloc instead of malloc to overwrite the allocated memory with zeroes, which in this case is the desired value (zero is interpreted as NULL/false).

You have more problems: The memory allocated on stack for num_words will be marked as unused on leaving the function, leaving you with pnum_words pointing to unused memory. Use a global unsigned int num_words, not a global unsigned int *pnum_words.

In "real" projects, you should always check the result of malloc or its relatives for being NULL. Most times, when it returns NULL stuff will just crash, but I would not want to rely on that.

1

You must log in to answer this question.

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