Hot answers tagged

6

Should I declare freenodes in the header file? It is not necessary to declare freenodes in the header file. However because freenodes is written after the function which calls it, it should be declared at the top of the .c file, like so: void freenodes(node* path); This is the same as the function signature used to define the function itself, with the ...


3

Totally awesome that you are asking the question even though it passed check50! Kudos. Notice this line Usage: speller [dictionary] text in the valgrind report. Speller didn't really run. You called valgrind but did not supply arguments to speller. Therefore, it didn't do anything, therefore no leaks. If you call valgrind with arguments to speller, you will ...


3

When implementing a recursive function, the base case should be written first because it's the first thing that should be executed to determine whether there should be one more recursive call or not! What you're doing, basically, is that you're executing the loop no matter whether the base case is reached and that's a bad practice and might sometimes cause ...


3

You have to free the data you allocated memory for separately within the nodes before you free the nodes themselves. For example, #include <stdio.h> #include <stdlib.h> int main(void) { // create a new struct struct Node { char *str; }; // create a new pointer struct Node *ptr = malloc(sizeof(struct Node)); ...


3

Complicated code in terms of memory management. This set of problems deals with the traditionally more difficult concepts of C programming, so we need a good theoretical basis for dealing with them. We must distinguish between a pointer that stores a memory address only, and the declaration of a new node in the linked list "node * tmp = malloc (sizeof (node)...


2

The cause you have pasted means that you are checking the value of a variable you didn't previously specify (most likely unloader -> children[i] != NULL). When creating a new node, you should initialize all children node pointers to NULL to make sure they don't contain any uninitialized, "garbage" memory.


2

So that error was being caused because the function getNode() was being used outside another function. Turns out, clang doesn't like that. So I did the following: node root_node; node* root = &root_node; That fixed it!


2

@SFri: I agree that it's hard to tell without seeing some code, but I'll tell you what i think can happen from the errors and the pseudocode. The reason to set a recently freed pointer to null is to clean up and be tidy, but it should not affect how your function works. If you get a double free error by commenting out the line that sets the freed pointer to ...


2

You overlooked something. Here's a hint. What is the integer value of ' - 'a'? Or, subtract the ASCII value of 'a' from the value of an apostrophe? I'm actually surprised that your code isn't generating an error. Oh, wait, it does, sort of. Try turning off the printing of misspelled words and run against a large text under valgrind --leak-checks=full. ...


2

The function free is used to free memory that was allocated on the heap using a function like malloc or calloc. You declared root as a global variable of type node. Global variables are stored on the data segment in memory not on the heap.


2

Ok, a couple of notes first to clear the field. Starting from easy: According to man 3 free: The free() function frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc(), or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behavior occurs. If ptr is NULL, no operation ...


2

Check that you've closed your dictionary file. Passing temp to free_memory() might not change temp to NULL. (Hopefully someone smarter can confirm via comments, or a better answer) bool unload(void) { node* temp = root; free_memory(temp); if ( temp == NULL) { return true; } else { return false; }...


2

It turned out one of my problems was I was freeing the root node after the recursive call, but the call itself frees the node, so that was causing a double free error. The actual error was that for the last letter, I was creating a new node as well as an ending node, and the new node was not getting picked up by the unload function.


2

one block of memory can be pointed at by many pointers. this is exactly what happens when you allocate memory for one pointer, then set the value of another pointer to be the same as the first pointer's value. example n = malloc(sizeof(node)); head = n; this allocates a block of memory on the heap, sets n to point to that block, then sets head to point to ...


2

Your code looks mostly good. Although you can return true from your recursive freeTrie() function and then in your unload() function just call freeTrie() on your root node. That's it. You don't need to worry about anything else because, for example, i expect your curr pointer is not malloc'd on the heap. It's just a node pointer in a function on the stack, ...


2

Your code isn't freeing recursively. It should keep on going down the tree while children[i]!=NULL, only freeing the node when that condition isn't met anymore. It then returns up the tree, freeing each node in turn. In your case, you are going down to the final children[i] then freeing spider, you're not going back up the tree.


2

Couldn't do a full, in-depth analysis because I would need to see the node struct declaration and all of the involved functions. However, here's a somewhat common issue. Look at the following code: for(int i = 0; i < 27; i++) { if(x->children[i] != NULL) { x = x->children[i]; NULL_search_and_free(x); } } Now, say that ...


2

Simply put, the unload function doesn't free anything. Worse, it loses the entire tree/trie. Here's why. First, pointer is created and initialized and set to NULL. Then the for loop starts. As the loop runs, each element in the hashtable array is set to NULL by this line: hashtable[i] = pointer;, losing track of the tree. Finally, the while loop never ...


2

Think carefully about what the code is doing. It checks if a pointer is null, and if so, it will free that node. In other words, it will free a nonexistent node - a do nothing operation. OR, if the node is not null, it will go on to process it's children, doing the same thing. BUT, on return, the node isn't freed. In other words, nothing is ever freed. ...


2

The nodes created here if (!create_new_node(palabra)) (one for each dictionary word) are never freed.


1

Did you make dictionary or make speller? The latter is correct, the former will produce the error you saw, along with about 20 relocation errors. This almost certainly has nothing to do with your code and all to do with how you tried to build the executable. Part of this lesson is understanding how to build a project using multiple source code files and a ...


1

@DinoCoderSaurus called it in the comments. Your destroy() function is calling itself recursively, and the comments indicate that you intend to free heap memory somewhere in there. Yet there is no actual call to free().


1

I believe what went wrong in your first clear function was an issue of scope. In your first function, you declared index outside of your while loop, so when the recursion kicks in, the loop increments that same index variable, instead of setting it back at 0. for loops work differently in that it will take care of that "resetting to 0" when it's used ...


1

Do not access new_node->next before testing for new_node == NULL. If the first one hasn't segfaulted, the second one won't match. Your error is related to the unload function. Your while loop will run at most once - after freeing exactly two memory blocks you return from the function. Move the return true; to after the outer loop, a return false; makes ...


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 ...


1

You have a tree structure (yes, a trie, but that's more a variation), and I don't see how you intend to free all nodes. One misconception you seem to have is that freeing something makes the pointer NULL (your head would never turn NULL with that code!) The easiest way to visit all nodes is to use recursion, or another kind of stack. That would require a ...


1

Function fopen() allocate memory for itself. Try without fclose(). You'll get leak of memory. You opened dictionary file and speller opened text file. So amount of memory will be = amount of nodes + amount of fopen() functions.


1

Prepare to slap head with hand! If you were to run valgrind with the full dictionary and a large file, you'd probably see thousands of blocks lost. So, given this declaration, typedef struct node { bool is_word; struct node* children[27]; } node; ... what's wrong with this code? void loop(void) { ... for (int i = 0; i < 26; i++) { ...


1

You free the node even if it has children. You should first destroy any non-NULL children, and then free the current node. No while loop required, just a single loop over all the children, calling destroy on the non-NULL ones.


1

Your problem arises when you read an apostrophe in load(). You have: c = fgetc(dict); if(isalpha(c) || c == '\'') { // Checks if the node already exists or not if(trav->children[c-96] == NULL) { trav->children[c-96] = createnode(); } trav = trav->children[c-96]; } But if c is an apostrophe ('), c-96 is 39-96 == -57 ...


Only top voted, non community-wiki answers of a minimum length are eligible