It seems that my program, though it compiles and begins to run, seg faults due to a memory leak acording to valgrind:
==4602== HEAP SUMMARY:
==4602== in use at exit: 4,007,248 bytes in 71,558 blocks
==4602== total heap usage: 71,564 allocs, 6 frees, 4,017,464 bytes allocated
==4602==
==4602== 336 bytes in 6 blocks are still reachable in loss record 1 of 4
==4602== at 0x307307F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4602== by 0x4019C1: check (dictionary.c:37)
==4602== by 0x40160B: main (speller.c:113)
==4602==
==4602== 336 bytes in 6 blocks are definitely lost in loss record 2 of 4
==4602== at 0x307307F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4602== by 0x4019F5: check (dictionary.c:44)
==4602== by 0x40160B: main (speller.c:113)
==4602==
==4602== 510,048 bytes in 9,108 blocks are definitely lost in loss record 3 of 4
==4602== at 0x307307F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4602== by 0x401BFA: load (dictionary.c:144)
==4602== by 0x4012CE: main (speller.c:40)
==4602==
==4602== 3,496,528 bytes in 62,438 blocks are still reachable in loss record 4 of 4
==4602== at 0x307307F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4602== by 0x401BFA: load (dictionary.c:144)
==4602== by 0x4012CE: main (speller.c:40)
==4602==
==4602== LEAK SUMMARY:
==4602== definitely lost: 510,384 bytes in 9,114 blocks
==4602== indirectly lost: 0 bytes in 0 blocks
==4602== possibly lost: 0 bytes in 0 blocks
==4602== still reachable: 3,496,864 bytes in 62,444 blocks
==4602== suppressed: 0 bytes in 0 blocks
here are the check and load funtions in which the leaks seem to be occuring, particularly at the lines in which memory is allocated:
bool check(const char *word)
{
// Hash word to check
unsigned int h = hash(word);
table[h] = malloc(sizeof(node));
if (table[h] == NULL)
{
return false;
}
// Set up cursor
node *cursor = malloc(sizeof(node));
if (cursor == NULL)
{
return false;
}
// Copy word to be checked into cursor
strcpy(cursor->word, word);
// Set cursor to point at first element of the linked list
cursor = table[h];
// Traverse linked list to check for word's presence
while (cursor->next != NULL)
{
if (strcasecmp(cursor->word, table[h]->next->word) == 0)
{
return true;
// Free allocated memory
while (cursor != NULL || table[h] != NULL)
{
node *tmp = cursor->next;
free(cursor);
cursor = tmp;
node *tmp2 = table[h]->next;
free(table[h]);
table[h] = tmp2;
}
}
else
{
// Move cursor to next node
cursor = cursor->next;
}
}
return false;
}
bool load(const char *dictionary)
{
// Open dictionary file
FILE *file = fopen(dictionary, "r");
// Check if opening was successful
if (file == NULL)
{
return false;
}
// Malloc new node
node *string = malloc(sizeof(node));
// Set up buffer, remember to initialize it, otherwise the compiler won't know what it points at
char read_file[LENGTH 1];
//Read every word
while (fscanf(file, "%s", read_file) != EOF)
{
// Scan word into read_file
fscanf(file, "%s", read_file);
// Check if memory was allocated correctly
if (string == NULL)
{
return false;
}
// Copy string from read file to node
strcpy(string->word, read_file);
// Hash the word and malloc
unsigned int h = hash(string->word);
table[h] = malloc(sizeof(node));
// Insert node into hash table
string->next = table[h]->next;
table[h]->next = string;
counter ;
}
// Close file
fclose(file);
// Free memory
free(string);
return true;
}
// Returns number of words in dictionary if loaded, else 0 if not yet loaded
unsigned int size(void)
{
return counter;
}
// Unloads dictionary from memory, returning true if successful, else false
bool unload(void)
{
// TODO
for (int i = 0; i < N; i )
{
node *cursor = NULL;
while (cursor != NULL)
{
cursor = table[i]->next;
node *tmp = cursor;
cursor = cursor->next;
free(tmp);
}
}
return true;
}
I will be happy to provide further information if it is needed.
CodePudding user response:
Just considering the allocations and the assignment:
table[h] = malloc();
cursor = malloc();
cursor = table[h]; // here you lose the only link to the newly allocated node!
// -> memory leak
Additionally you do not check if there's already a node in table[h]
then you lose the link to it as you just overwrite without checking.
My personal approach would be first looking for the insertion point, then create the new node:
node** cursor = &table[h];
while(*cursor)
{
cursor = &(*cursor)->next;
}
*cursor = malloc(sizeof(**cursor));