0

I have spent more than 30 hours on this problem set and I am still struggling. At first, my program compiled but did not run, due to a segmentation fault. I finally fixed it and everything works fine, except that the program is suffering from memory leaks. Valgrind had been so mad, and I wonder if someone may give me some ideas. Attached below is my code. Any help is appreciated, thank you!

/**
 * dictionary.c
 *
 * Computer Science 50
 * Problem Set 5
 *
 * Implements a dictionary's functionality.
 */

#include <cs50.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <ctype.h>
#include "dictionary.h"

int word_count = 0;

// creation of node
typedef struct node
{
    char word_dict[LENGTH + 1];
    struct node *next;
}
node;

// creation of hash table and definition of hash function (reference: http://cs50.stackexchange.com/questions/9460/pset5-speller-c-check)
node* hashtables[26];
int hash_function (const char *key)
{
    int index = (toupper(key[0]) - 'A') % 26;
    return index;
}

/**
 * Returns true if word is in dictionary else false.
 */
bool check (const char* word)
{
    // satisfy case-insensitive requirement by changing all words prompted by the user into purely small caps
    char* small_cap = NULL;
    char* small_cap malloc(sizeof(strlen(word)));
    for (int i = 0; i < (strlen(word)); i++)
    {   
        if (isupper(word[i]))
            small_cap[i] = tolower(word[i]);
        else
            small_cap[i] = word[i];
    }

    // hash the word prompted by th user and obtain the resulting index
    int index_text = hash_function(small_cap);

    // string-compare the word entered by the user with words from the dictionary
    node* cursor = malloc(sizeof(node));
    cursor = hashtables[index_text];
    if (index_text >= 0)
    {
        while(cursor != NULL)
        {
            if(strcmp(cursor -> word_dict, *small_cap) == 0)
                return true;
            else
                cursor = cursor -> next;
    }
    }
    return false;
}

/**
 * Loads dictionary into memory.  Returns true if successful else false.
 */
bool load (const char* dictionary)
{
    // open the dictionary, as per the user, in the read mode
    FILE* fp_dict = fopen(dictionary, "r");

    // make sure the dictionary is opened successfully
node* head = NULL;
    if (fp_dict == NULL)
    printf("Unable to open dictionary\n");

    // perform a while loop whenever it is not the end of the dictionary yet
    while(!feof(fp_dict))
    {
       // make a new pointer pointing to a new word from the dictionary and scan the word this pointer points to
       node* new_node = malloc(sizeof(node));
       fscanf(fp_dict, "%s", new_node -> word_dict);

       // hash the word from the dictionary and obtain the resulting index
       int hash = hash_function(new_node -> word_dict);

       // make sure hash is an integer equally to or larger than zero
       if (hash >= 0)
       {
           // the following condition returns true when no word_dict has been linked to the index of interest yet, or in other words, when the bucket of interst is empty
           if (hashtables[hash] == NULL)
           {
               hashtables[hash] = new_node;
               head = hashtables[hash];
               head -> next = NULL;
               word_count++;
           }
           else // hashtables[hash] != NULL, or in other words, the "bucket" of interest is not empty
           {
               new_node -> next = head;
               head = new_node;
               word_count++;
           }
           // note that in either case above, the new node is always inserted to the very beginning to the linked list
       }
    }

    // once the end of the dictionary is reached
    if (feof(fp_dict))
    {
        return true;
        fclose(fp_dict);
    }

    return false;
    fclose(fp_dict);
}

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

/**
 * Unloads dictionary from memory.  Returns true if successful else false.
 */
bool unload(void)
{
    int unload_check = 0;
    // the while loop frees linked lists one by one, down the hash table
    while (unload_check < 26)
    {
        node* unload_cursor = hashtables[unload_check];
        if (unload_cursor == NULL)
        {
            unload_check++;
        }
        else
        {
            while (unload_cursor != NULL)
            {
                node* temp = unload_cursor;
                unload_cursor = unload_cursor -> next;
                free (temp);
            }
            unload_check++;
        }
    }
    return true;
}
0

You have done the equivalent of painting yourself into a corner. When you've reached the end of the list for a particular hashtable[unload_check], you rightly free temp but what about the node you're currently pointing to?

| improve this answer | |
  • I have tried to free unload_cursor at the end too but it did not seem the right thing to do. Please provide more guidelines as to how to stop memories from leaking. Thank you so much!!!!!! – Alex Chan Oct 31 '16 at 0:29
  • Well, keep that line in there, you should free unload_cursor after you free temp. Also, instead of while (unload_cursor != NULL) in your unload() function, try while (unload_cursor->next != NULL). – ronga Nov 1 '16 at 11:52

You must log in to answer this question.

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