0

Hi) I understand that something incorrect with unload function and that 143,091 blocks it's basically the number of words in the dictionary that was uploaded by the program and I didn't manage to unload. If possible could someone help me to solve this memory leak question?

// Implements a dictionary's functionality

#include <stdbool.h>
#include <cs50.h>
#include <strings.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

#include "dictionary.h"
#define MULTIPLIER (37)

// Represents a node in a hash table
typedef struct node
{
    char word[LENGTH + 1];
    struct node *next;
}
node;

// Number of buckets in hash table
const unsigned int N = 26;
unsigned int hash_value;
unsigned int word_count;
// Hash table
node *table[N];

// Returns true if word is in dictionary, else false
bool check(const char *word)
{
    hash_value = hash (word);
    node *cursor = table[hash_value];

    while (cursor != NULL)
    {
        if (strcasecmp(word, cursor -> word) == 0)
        {
            return true;
        }
        cursor = cursor -> next;
    }
    return false;
}

// Hashes word to a number
// Taken from http://www.cs.yale.edu/homes/aspnes/classes/223/notes.html#hashTables
unsigned int hash(const char *word)
{
    unsigned long h;
    unsigned const char *us;

    us = (unsigned const char *) word;

    h = 0;
    while(*us != '\0')
    {
        h = tolower(h * MULTIPLIER + *us);
        us++;
        return h;
    }
    return 0;
}

// Loads dictionary into memory, returning true if successful, else false
bool load(const char *dictionary)
{
    FILE *file = fopen(dictionary, "r");
    if (file == NULL)
    {
        return false;
    }
    char word [LENGTH + 1];

    node *n = NULL;
    while (fscanf(file, "%s", word) != EOF)
    {
        n = malloc(sizeof(node));
        n->next = NULL;
        if (n == NULL)
        {
            return false;
        }

        strcpy (n->word, word);
        hash_value = hash(word);
        n->next = table [hash_value];
        table [hash_value] = n;
        word_count ++;
    }

    fclose(file);
    return true;
}

// Returns number of words in dictionary if loaded, else 0 if not yet loaded
unsigned int size(void)
{
    if (word_count > 0 )
    {
        return word_count;
    }
    return 0;
}

// Unloads dictionary from memory, returning true if successful, else false
bool unload(void)
{
    for (int i = 0; i < N; i++)
    {
        node *cursor = table[i];
        node *tmp = cursor;

        while (cursor != NULL)
        {
            cursor = cursor -> next;
            free (tmp);
        }
        if (i == N - 1 && cursor == NULL)
        {
            return true;
        }
    }
    return false;
}

Here is what Valgrind said to me after changing while (cursor != 0) to while (cursor != NULL):

==385== 
==385== HEAP SUMMARY:
==385==     in use at exit: 8,013,096 bytes in 143,091 blocks
==385==   total heap usage: 143,096 allocs, 5 frees, 8,023,256 bytes allocated
==385== 
==385== 8,013,096 bytes in 143,091 blocks are still reachable in loss record 1 of 1
==385==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==385==    by 0x401AFA: load (dictionary.c:78)
==385==    by 0x4012BE: main (speller.c:40)
==385== 
==385== LEAK SUMMARY:
==385==    definitely lost: 0 bytes in 0 blocks
==385==    indirectly lost: 0 bytes in 0 blocks
==385==      possibly lost: 0 bytes in 0 blocks
==385==    still reachable: 8,013,096 bytes in 143,091 blocks
==385==         suppressed: 0 bytes in 0 blocks
==385== 
==385== For lists of detected and suppressed errors, rerun with: -s
==385== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
2
  • Could you add the whole valgrind error output
    – Vsjain
    Mar 18 at 5:35
  • Added) this is after applying changes to code. Mar 18 at 10:53
0

In your unload function, you check if the cursor is zero. Instead you should check if the cursor is NULL. So the condition for the while loop should be cursor != NULL. The valgrind error states line 77 in dictionary.c, which is node *n = malloc(sizeof(node)). To fix this problem, declare n outside the while loop as node *n = NULL. Then inside the loop, do n = malloc(sizeof(node)). On the next line, set n->next to NULL. This should solve your problem.

Hope this helps. If it does then please check the tickmark.

1
  • Sorry man. I've changed the code as you suggest (also updated the post), but ((( Anyway, thank you for your support and help. I will continue to investigate the issue of this memory leak. Mar 18 at 14:36

You must log in to answer this question.

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