0

I have a problem here, if I fclose(dict_source) (comment where I place it) all code stop working (code stop find misspelled words), also if I try free(index) it's return free(): invalid pointer. Other test, except memory program is passing, but if you see other problem feel free to point me there. Thank you for help.

// Implements a dictionary's functionality

#include <stdbool.h>
#include <stdio.h>
#include "dictionary.h"
#include <stddef.h>
#include <ctype.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
// 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 = 99990000;
int count = 0;

// Hash table
node *table[N] = {NULL};

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    unsigned int bucket = hash(word);
    node *pointer;
    pointer = table[bucket];

    while (pointer != NULL)
    {
        if (strcasecmp(pointer->word, word) == 0)
        {
            free(pointer);
            return true;
        }
        else
        {
            pointer = pointer->next;
        }
    }
    //free(pointer);
    return false;
}
// http://www.cse.yorku.ca/~oz/hash.html
//djb2 with check for word is here and drop value 999900000 max, edit if needed
unsigned int hash(const char *word)
{
    if ( word == NULL)
    {
        return 0;
    }
    char *index = malloc(46);
    // make shure we will have same hash for lower and upper
    for (int i = 0; word[i] != '\0'; i++)
    {
        index[i] = tolower(word[i]);
    }
    unsigned int hash = 5381;
    int c;

    while ((c = *index++))
    {
        hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
    }
    while(hash > 99989999)
    {
        hash = hash / 10;
    }
    //printf("hash is:%i", hash);
    //free(index);
    return hash;
}


// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    char d_word[46];
    FILE *dict_source = fopen(dictionary, "r");
    if (dict_source == NULL)
    {
        return false;
    }
    while (!feof(dict_source))
    {
        count++;

        //fgets (d_word, 45, dict_source);
        fscanf(dict_source, "%s", d_word);
        //printf ("%s", d_word);
        int d_hash = hash(d_word);
        node *new_word = malloc(sizeof(node)); // we can malloc less memory, but here it's not the case
        if (new_word == NULL)
        {
            printf(" can't malloc for new_word");
            return 0;
        }
        strcpy(new_word -> word, d_word);
        new_word -> next = NULL;
        if (table[d_hash] == NULL)// check if 0 word in this table bucket
        {
            table[d_hash] = new_word;
        }
        else
        {
            new_word -> next = table[d_hash] -> next; // copy first pointer
            table[d_hash] -> next = new_word; // change table pointer to point into new node
        }



    }

    //fclose(dict_source);
    return true;
    // TODO
    //return false;
}

// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
    // TODO
    return count - 1;
}

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

        while (pointer1 != NULL)
        {
            pointer2 = pointer1;
            pointer1 = pointer1 -> next;
            free(pointer2);
        }

    }
    // TODO
    return true;
}

Ok, so after my sleep I can handle it with other approach and brute force. Rewrite hash function, delete free(pointer) at check() , after this fclose(dict_sourse) start working and all leaks are closed. Still can't understand problem, but manage to avoid it.

    // Implements a dictionary's functionality

#include <stdbool.h>
#include <stdio.h>
#include "dictionary.h"
#include <stddef.h>
#include <ctype.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
// 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 = 27;
int count = 0;

// Hash table
node *table[N] = {NULL};

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    unsigned int bucket = 0;
    bucket = hash(word);
    node *pointer;
    pointer = table[bucket];

    while (pointer != NULL)
    {
        if (strcasecmp(pointer->word, word) == 0)
        {
            // delete here free(pointer); it's somehow block to fclose(dict_sourse) at 109; 
            return true;
        }
        else
        {
            pointer = pointer->next;
        }
    }
    //free(pointer);
    return false;
}

// rewrite hash function, so now I can understand it 
unsigned int hash(const char *word)
{
    if ( word == NULL)
    {
        return 0;
    }
    unsigned int hash;
    if (isupper(word[0]))
    {
        hash = (word[0]) -'A';
    }
    else
    {
        hash = word[0] - 'a';
    }
    return hash;
}


// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    char d_word[46];
    FILE *dict_source = fopen(dictionary, "r");
    if (dict_source == NULL)
    {
        return false;
    }
    while (!feof(dict_source))
    {
        count++;

        //fgets (d_word, 45, dict_source);
        fscanf(dict_source, "%s", d_word);
        //printf ("%s", d_word);
        int d_hash = hash(d_word);
        node *new_word = malloc(sizeof(node)); // we can malloc less memory, but here it's not the case
        if (new_word == NULL)
        {
            printf(" can't malloc for new_word");
            return 0;
        }
        strcpy(new_word -> word, d_word);
        new_word -> next = NULL;
        if (table[d_hash] == NULL)// check if 0 word in this table bucket
        {
            table[d_hash] = new_word;
        }
        else
        {
            new_word -> next = table[d_hash] -> next; // copy first pointer
            table[d_hash] -> next = new_word; // change table pointer to point into new node
        }



    }

    fclose(dict_source);
    return true;
    // TODO
    //return false;
}

// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
    // TODO
    return count - 1;
}

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

        while (pointer1 != NULL)
        {
            pointer2 = pointer1;
            pointer1 = pointer1 -> next;
            free(pointer2);
        }

    }
    // TODO**
    return true;
}

2 Answers 2

1

A couple of issues with your code:

In your 'check' function you are trying to free() memory from a pointer in which you didn't allocate any memory. In fact, as it is pointing to the word found in your hash list, it will delete that node from the table, losing access to the rest of the elements.

In your 'hash' function you did reserve memory here: char *index = malloc(46);, but later on, you increment the position of the pointer here: while ((c = *index++)). By doing that index is not pointing to the begining of the memory you reserved so it will give you an error message when you try to free() it. Also by copying the lower word here: for (int i = 0; word[i] != '\0'; i++), you're not including the '\0' character at the end of index[] which will give you trouble.

About: fclose(dict_source); it didn't throw any error to me when I tried it. Maybe you could post some information about that error like number of line etc....

1
  • Yes, you are right, fclose() don't make error, but completely stop find misspelled words. In first version. Apr 27, 2020 at 12:20
0

remove the pointer 2 in unload function it is irrelevant, the index is a pointer to your values so what you do is you set pointer1 = table[i] then free table[i], then table[1] = pointer1->next;.. In your load function you don't have to know if the index you have has no words yet. you can just make an algorithm that whenever you get a new word, that new world will now be the first one in the list by just manipulating the pointers you have,

strcpy(newNode, words);
newNode->next = table[index];
table[index] = newNode;

I'm unsure why your fclose function makes your program crash, try to debug your code and reread your code to check your logics, but fclose is necessary because if you don't close the file you will get memory leaks...

You must log in to answer this question.

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