I have the speller up and running, working properly and passing the cs50 automatic verification, but, independently of the size of the dictionary (even with a one word dictionary or an empty one) valgrind always tells me that I have one block that is definitely lost (52 bytes).

The leak is in the load function, precisely in the line that loads the dictionary into memory, which goes like this:

for (node* new_node = malloc(sizeof(node)); fscanf(dict, "%s",new_node->word) == 1; new_node = malloc(sizeof(node)))

Clearly this line is mallocing one node to many and I think I will have to create a temporary variable before the loop to temporary hold the word from the dictionary and then, inside the loop, copy it into the new_node->word.

But is there anyway I could make this work? It seems so much cleaner without having to use a temp variable.




You can try this:

node* new_node;
for (new_node = malloc(sizeof(node)); fscanf(dict, "%s",new_node->word) == 1; new_node = malloc(sizeof(node)))
    // your stuff here

See that I declared node* new_node outside of the for loop so that it doesn't go out of scope when the loop exits. Then you just simply free the last new_node you have allocated.

I don't like that I made new_node accessible to a larger scope, but if you want to keep it the way it is I think this will work.

If this answers your question please accept it by clicking the gray check-mark to the left, so that it becomes green. You can also vote it up by pressing the up arrow above the check-mark. And don't forget to keep coding!

  • 1
    Worked like a charm! Thanks! Not much difference in the loading time, but the code looks smarter :)
    – José
    Sep 11 '15 at 20:08

You must log in to answer this question.

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