5
Actually, your apparent problem is that you are running valgrind against a different executable binary, also called resize.
~/workspace/ $ which resize
/usr/bin/resize
~/workspace/ $ whatis resize
resize (1) - set environment and terminal settings to current xterm window size
Try running valgrind against ./resize (in the appropriate pset ...
3
for (int i = 0; i < 26; i++)
You have 27 elements (a-z and '), so <=26 or <27 would be appropriate.
3
I'm not sure you needed to learn about pointers and memory management in order to solve hacker 2. I think you've gone a little bit far. However, I'm gonna try answering your questions.
A pointer is basically a variable that stores an address of a location in memory. Pretty much every type has its own pointer type. For example, char has char *, int has int *,...
2
I am also a newbie at programming but hope I can help. I've been stuck with the same problem for some time and just solved with some help.
The 52 bytes is the size of one node not freed which in this case is your entry node.
Instead of using an entry node, declare a temporary string before the while loop. assign the input from fscanf to temporary string. ...
2
This question was also posed on Facebook by you here: https://www.facebook.com/groups/cs50/permalink/343562319124153/
In a nutshell, GetString() mallocs space to hold the character array that you then enter. Any "definitely lost" bytes that valgrind reports are from that GetString() malloc unless you specifically free it using free(s); or free(ptr->str)...
2
You have only implemented part of your goal. This code will walk down a linked list to the last element, but will only free memory when it finds that the next element is null, i.e., it is at the end of the list.
To free memory as you go (beginning to end), you should create a temp pointer to hold the address of the next element, and then free the current ...
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
TL; DR:
This looks like a memory leak.
Remove:
node *tableindex = malloc(sizeof(node));
tableindex = hashtable[ind];
...
free(tableindex);
Add:
node *tableindex = hashtable[ind];
comparison of the original and updated code (I took some guesses at your hash function etc..)
Why:
It seems the main problem is how the node pointer tableindex is ...
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
Yes and no. Yes it is stored to memory, but the way in which it is declared has no memory leaks (try making a test file and running valgrind to see for yourself).
What does cause a memory leak is when you allocate memory for a variable using malloc (or similar) and don't subsequently free that memory. For example:
void print_hello_twice()
{
char* hello ...
2
Why are you mallocing space in check?
Also, your free(checker) only runs if the word is not found. How about when the word is found? You return true and the function ends, so that free line is never reached.
2
Without seeing more code, it's hard to know. A leak of 568 bytes in 1 block usually indicates a file that wasn't closed. Is it possible that there's a return command executed before the dictionary file was closed?
If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)
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
You don't free the realloced memory. Your code
buffer_temp_word = NULL;
free(buffer_temp_word);
means free is called for NULL. You should not change the value of buffer_temp_word there, you could increment a copy instead, or use an index variable.
And 8 bytes is correct, in both cases you lose the memory when you are at a char_count ...
2
Nothing after "MISSPELLED WORDS" in the check50 log indicates a segmentation fault. This line in unload
while (cursor == NULL)
is the culprit. It cannot free NULL. The equality test is wrong.
The problem with cursor in check is that it does not have to be malloc'd at all. It is node pointer only, not an actual node (it doesn't hold a word or next). A ...
2
You are getting that error because in your 'load' function, you are inicializing the value 'next' here: temp->next = table[index]; inside an 'if' condition. If you also set temp->next to NULL after you declare it, you'll be good to go.
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
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
fclose the dict at the end of load, the file should be open only for the duration of reading. Otherwise, unload looks fine.
In your hash function
return (c != '\'')? c - 'a' : CHILDREN_LENGTH;
should be
return (c != '\'')? c - 'a' : CHILDREN_LENGTH - 1;
because CHILDREN_LENGTH - 1 is the highest index in an array of length CHILDREN_LENGTH.
Then ...
1
If you free the root node in load, you cannot use it in check. You allocate "lots of " child nodes, but only "free" the last one. You absolutely can test that load works before implementing unload, and I daresay you should. (NB the best way to test that load works, is to write check).
Do all the freeing in unload. Remember, each node's address is stored in ...
1
In check:
wordcopy = malloc(wordlength * sizeof(char)); allocates too little space, should be malloc((wordlength+1) * sizeof(char));, and don't forget the wordcopy[wordlength] = '\0'; null terminator, cause of the invalid read
you create a new node in cursor = malloc(sizeof(node));, but throw away the pointer using cursor = hashtable[hashkey]; (remove that ...
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* ...
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 ...
1
Does valgrind return the leak if it finds index.html or index.php? I would predict this indexes would leak memory only if neither of those files is "found". Generally speaking, one would not change anything about "main" for this assignment, so adding a free to main is not the right approach. Since the memory for pathCopy is allocated in the indexes function, ...
1
I wanted to test your code so I took it from the link you provided.
When I tried to compile it, it did not compile. I had to fix two lines to get it to compile:
Line numbers are based on the code you provided:
Line 41 and 42:
char* small_cap = NULL;
char* small_cap malloc(sizeof(strlen(word)));
I did not understand why make it NULL if you were assigning ...
1
Take a close look at the code for free_node()
for (int i = 0; i < 27; i++)
{
if (cursor -> children[i] != NULL)
{
cursor = cursor -> children[i];
return free_node(cursor);
}
It reassigns cursor before recursively calling free_node(). But what happens when it returns to this level? cursor is no longer pointing at the ...
1
Yes it's true that for simplicity's sake, in the first psets, we din't free() the strings allocated by GetString() and that caused memory leaks.
Nevertheless, most modern operating systems can see when a program exists with memory leaks, and frees the memory itself. So even if you run your program a million times you would not notice any memory shortage (...
1
You can use your recursive function, but to do so, you need to pass the FILE* target variable by reference, rather than by value as you have done.
Recall Prof Malan's noswap.c example in Week 3. Or perhaps as an even simpler example, this:
#include <stdio.h>
void addOne(int num);
int main(void)
{
int num = 99;
printf("num before: %d\n", num);...
1
Your valgrind results:
==3069== at 0x402A17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
x86-linux.so)
==3069== by 0x8049087: check (dictionary.c:59)
==3069== by 0x8048B6D: main (speller.c:117)
==3069==
show that you are mallocing nodes in check. Your check function should not be mallocing any nodes. It should simply be looking through ...
Only top voted, non community-wiki answers of a minimum length are eligible
