0

I have a memory leak. Valgrind told me that I had over 10000000 errors. So what is the problem? Someone told me there was no problem in my unload function, so here's my Load function.

bool load(const char* dictionary) { FILE* file = fopen(dictionary,"r"); if (dictionary == NULL) { return false; } node* new_word = malloc(sizeof(node)); while (!feof(file)) { fscanf(file,"%s\n",new_word->word); if (hashtable[hash(new_word->word)] == NULL) { hashtable[hash(new_word->word)] = new_word; new_word->next = NULL; } else { new_word -> next = hashtable[hash(new_word->word)]; hashtable[hash(new_word->word)] = new_word; } count++; } free(new_word); return true; fclose(file); }

I know it is a problem with freeing the node new_word. This is an edit I made to try and free it. Before editing, node new_word was being declared inside the while loop, and I didn't try to free new_word.

1

I think you should return false if file is NULL, not just dictionary (the latter is highly unlikey given the surrounding code, the former might happen if the file is not where you expect it to be)

From your code, I'd assume you had more of a double-free or infinite loop problem, as you malloc the space for your string outside the loop, instead of

node* new_word = malloc(sizeof(node));
while (!feof(file))
{
    fscanf(file,"%s\n",new_word->word);

I'd expect something like

while (!feof(file))
{
    node* new_word = malloc(sizeof(node));
    fscanf(file,"%s\n",new_word->word);

And

    if (hashtable[hash(new_word->word)] == NULL)
    {
        hashtable[hash(new_word->word)] = new_word;
        new_word->next = NULL;
    }
    else 
    {
        new_word -> next = hashtable[hash(new_word->word)];
        hashtable[hash(new_word->word)] = new_word;
    }

can be abbreviated to just

    new_word -> next = hashtable[hash(new_word->word)];
    hashtable[hash(new_word->word)] = new_word;

(BTW, you might want to cache the result of hash(new_word->word), as you call it multiple times while the result should be all the same)

And as you would have created a node structure for each entry, you don't want to free that at the end of the function.

Oh, and code after return true; cannot be reached, so you don't close the file.

1

568 bytes in 1 block is usually an open file pointer that hasn't been closed.

As for the rest, I can't tell from the code posted, or more specifically, not posted. The posted code looks like it should work fine. I'm thinking that there's more likely a problem in load.

A common error is to declare a pointer with a malloc and then to reassign it, thus orphaning the malloc'd memory. For example:

node* alpha = malloc(sizeof(node));
node* beta= malloc(sizeof(node));
beta = alpha ;

This would assign the address of the first malloc'd memory to both alpha and beta, but would abandon the second malloc'd memory, creating a memory leak.

If this isn't the problem, then I would suggest editing the question, adding the code for load() and any other code that contains a malloc, calloc, or any of it's cousins.

[EDIT: following question edit.]

Blauef covers a lot. I would emphasize that in your load code, a new node is only malloc'd once, so you're reusing the same node over and over, and freeing it at the end. No tree is being built.Just a series of long linked lists that all point to the same node. Worse, that single node is released by the free() statement at the end. A new node needs to be malloc'd with each word. Note that the same pointer, new_word, can be reused to store the address of each node as you process each new word. Once a new word node has been added to a linked list, it is safe, so new_word can be updated to the next node's address.

Memory leak: I'm still not seeing the cause of the memory leak. Since you're reusing a single node allocated by a single malloc, the memory is being allocated elsewhere. It must be somewhere else in your code.

In any case, you have a lot of rewriting to do. Happy coding!

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

This site is temporarily in read only mode and not accepting new answers.

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