Reputation: 65
I have read other threads on pset5 Valgrind memory errors, but that didn't help me. I get 0 leaks, but this instead:
==1917== Conditional jump or move depends on uninitialised value(s) Looks like you're trying to use a variable that might not have a value? Take a closer look at line 34 of dictionary.c.
The error refers to line 34 which is this: lower[i] = tolower(word[i]);
To supply context, the code below attempts to check if a word exists in the dictionary that has been uploaded to a hash table. I am attempting to convert the wanted word to lowercase because all the dictionary words are also lowercase and so that their hashes would be identical. The program successfully completes all tasks, but then stumbles upon these memory errors.
Any hints as to why Valgrind is mad at me? Thank you!
// Returns true if word is in dictionary else false
bool check(const char *word)
{
char lower[LENGTH + 1];
//Converts word to lower so the hashes of the dictionary entry and searched word would match
for (int i = 0; i < LENGTH + 1; i++)
{
lower[i] = tolower(word[i]);
}
// Creates node from the given bucket
node *tmp = table[hash(lower)];
// Traverses the linked list
while (tmp != NULL)
{
if (strcasecmp(word, tmp->word) == 0)
{
return true;
}
tmp = tmp->next;
}
return false;
}
Below is the whole dictionary.c file:
// Implements a dictionary's functionality
#include <string.h>
#include <strings.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include "dictionary.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 26^3
const unsigned int N = 17576;
// Hash table
node *table[N];
int count = 0;
// Returns true if word is in dictionary else false
bool check(const char *word)
{
char lower[LENGTH + 1];
//Converts word to lower so the hashes of the dictionary entry and searched word would match
for (int i = 0; i < LENGTH + 1; i++)
{
lower[i] = tolower(word[i]);
}
// Creates node from the given bucket
node *tmp = table[hash(lower)];
// Traverses the linked list
while (tmp != NULL)
{
if (strcasecmp(word, tmp->word) == 0)
{
return true;
}
tmp = tmp->next;
}
return false;
}
// Hashes word to a number
unsigned int hash(const char *word)
{
// Modified hash function by Dan Berstein taken from http://www.cse.yorku.ca/~oz/hash.html
unsigned int hash = 5381;
int c;
while ((c = *word++))
{
hash = (((hash << 5) + hash) + c) % N; /* hash * 33 + c */
}
return hash;
}
// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
FILE *inptr = fopen(dictionary, "r");
if (dictionary == NULL)
{
printf("Could not load %s\n.", dictionary);
return false;
}
// Create a char array to temporarily hold the new word (r stands for read)
char r_word[N+1];
// Until the end of file
while (fscanf(inptr, "%s", r_word) != EOF)
{
// Increments count
count++;
// Create a node
node *new_node = malloc(sizeof(node));
if (new_node == NULL)
{
unload();
return false;
}
strcpy(new_node->word, r_word);
// Hash the node
int index = hash(new_node->word);
// Places the node at the right index
new_node->next = table[index];
table[index] = new_node;
}
fclose(inptr);
return true;
}
// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
if (&load == false)
{
return '0';
}
else
{
return count;
}
}
// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
// Interates over the array
for (int i = 0; i < N; i++)
{
node *head = table[i];
while (head != NULL)
{
node *tmp = head;
head = head->next;
free(tmp);
}
}
return true;
}
Upvotes: 0
Views: 184
Reputation: 5615
This loop iterates through the maximum length of word
-
for (int i = 0; i < LENGTH + 1; i++)
{
lower[i] = tolower(word[i]);
}
Except if you look at how word
is created-
while (fscanf(inptr, "%s", r_word) != EOF)
{
// Increments count
count++;
// Create a node
node *new_node = malloc(sizeof(node));
if (new_node == NULL)
{
unload();
return false;
}
strcpy(new_node->word, r_word);
Notice, the variable r_word
, may not be exactly of length LENGTH + 1
. So what you really have in word
is N number of characters, where N is not necessarily LENGTH + 1
, it could be less.
So looping over the entire 0 -> LENGTH + 1
becomes problematic for words that are shorter than LENGTH + 1
. You're going over array slots that do not have a value, they have garbage values.
What's the solution? This is precisely why c strings have \0
-
for (int i = 0; word[i] != '\0'; i++)
{
lower[i] = tolower(word[i]);
}
This will stop the loop as soon as the NULL character is reached, which, you must have already learnt, marks the end of a string - aka a char array.
There may still be more errors in your code. But for your particular question - reading out of bounds is the answer.
Upvotes: 1