Nichael18
Nichael18

Reputation: 73

Cs50 pset5 speller segmentation, memory errors

I'm doing the speller program and for the past few days I feel like I'm going in circles, receiving the same problems over and over again. Now, the error is free(): "invalid pointer Aborted", most likely refering to the hash function, in which I use calloc,yet I can't understand what am I doing wrong there. Most likely there are multiple mistakes in my other functions, so tips regarding them would be very appreciated. I'll post the entire program, so I don't have to send snippets of it later. Completely lost here. Ok, thanks to Tim Randal's answer the free() error looks fixed, however a segmentation error mentioned previously took his place.

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "dictionary.h"
#include <strings.h>
#include <ctype.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 = 1000;

// Number of words loaded into the dictionary
int counter = 0;
// Hash table
node *table[N] = {0};

// Returns true if word is in dictionary, else false
bool check(const char *word)
{
size_t len = strlen(word);
char *lower = calloc(len+1, sizeof(char));

for (size_t i = 0; i < len; ++i) {
    lower[i] = tolower((unsigned char)word[i]);
}
node *cursor = NULL;
cursor = table[hash(lower)];
while (cursor != NULL)
{
    if (strcasecmp(cursor->word,lower) == 0)
    {
        free(lower);
        return true;
    }
    else
    {
        cursor = cursor->next;
    }
}
free(lower);
return false;
}

// Hashes word to a number
//hash function djb2 by Dan Bernstein
unsigned int hash(const char *word)
{
    size_t len = strlen(word);
char *lower = calloc(len+1, sizeof(char));

for (size_t i = 0; i < len; ++i) 
{
    lower[i] = tolower((unsigned char)word[i]);
}
    unsigned long hash = 5381;
    int c;

    while ((c = *lower++))
        hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
    free(lower);
    if (hash > N)
    {
        hash = hash % N;
        return hash;
    }
    else
    {   
        return hash;
    }

 }

 // Loads dictionary into memory, returning true if successful, else false
 bool load(const char *dictionary)
{
char word[LENGTH + 1];
node *new_node = NULL;
node *tmp = NULL;
FILE *file = fopen(dictionary,"r");
if (file == NULL)
{
    printf("Could not open file\n");
    return false;
}
while(fscanf(file, "%s",word) != EOF)
{
    new_node = malloc(sizeof(node));
    if (new_node == NULL)
    {
        printf("Not enough memory!\n");
        return false;
    }
    strcpy(new_node->word,word);
    unsigned int index = hash(word);
    
    if(table[index] == NULL)
    {
        table[index] = new_node;  
        new_node->next = NULL;
    }
    else
    {
    tmp = table[index];
    table[index] = new_node;
    new_node->next = tmp;
    }
    counter++;
    


}
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)
{
const char *word = NULL;
int index = hash(word);
for(node *cursor = table[index]; cursor != NULL;cursor = cursor->next)
{
    for(node *tmp = table[index]; tmp != NULL; tmp = tmp->next)
    {
        free(tmp);
    }
    free(cursor);
}   
return true;
}

Upvotes: 3

Views: 87

Answers (1)

Tim Randall
Tim Randall

Reputation: 4145

Here's one problem. I'm not saying it's the only one.

while ((c = *lower++))
    hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
free(lower);

You're freeing a pointer after changing it. You will need to copy lower to another pointer, which gets incremented and tested. The original lower needs to remember the address of the first byte allocated.

char* test = lower;
while ((c = *test++))
    hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
free(lower);

Upvotes: 2

Related Questions