0

I posted something similar a couple of days ago but have made progress since then. Specifically I found a couple of posts answered by Cliff B indicating the problem users had was that they hadn't initialized the table[i]. I have done this with a loop at the start of the load function, but that hasn't solved the problem.

So here's what Valgrind is saying:

==3417== Conditional jump or move depends on uninitialised value(s)
==3417==    at 0x40103D: check (dictionary.c:31)
==3417==    by 0x400C6B: main (speller.c:112)
==3417==  Uninitialised value was created by a heap allocation
==3417==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3417==    by 0x40116D: load (dictionary.c:74)
==3417==    by 0x40095E: main (speller.c:40)
==3417== 
A
is
not
a
==3417== Conditional jump or move depends on uninitialised value(s)
==3417==    at 0x401281: unload (dictionary.c:117)
==3417==    by 0x400DDF: main (speller.c:152)
==3417==  Uninitialised value was created by a heap allocation
==3417==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3417==    by 0x40116D: load (dictionary.c:74)
==3417==    by 0x40095E: main (speller.c:40)
==3417== 

WORDS MISSPELLED:     4
WORDS IN DICTIONARY:  5
WORDS IN TEXT:        6
TIME IN load:         0.02
TIME IN check:        0.00
TIME IN size:         0.00
TIME IN unload:       0.00
TIME IN TOTAL:        0.03

==3417== 
==3417== HEAP SUMMARY:
==3417==     in use at exit: 0 bytes in 0 blocks
==3417==   total heap usage: 10 allocs, 10 frees, 10,600 bytes allocated
==3417== 
==3417== All heap blocks were freed -- no leaks are possible
==3417== 
==3417== For counts of detected and suppressed errors, rerun with: -v
==3417== ERROR SUMMARY: 5 errors from 2 contexts (suppressed: 0 from 0)

And below is my code. Valgrind refers to three lines (31, 117, and 74). As far as I understand Valgrind, it's saying that in line 31 and 117 I have something depending on uninitialized values, AND that the unitialized value in both cases was created by malloc in the load function at line 74. (Please correct me if this is wrong)

So I've marked those lines with comments below (line 74 is simply marked with MANIC LAUGHTER :)). I would really, really appreciate any help on this one, as I've been banging my head against a wall for a few days on this one! :)

// Implements a dictionary's functionality

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.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;

int wordcount;

// Number of buckets in hash table (i.e. letters in the alphabet)
const unsigned int N = 26;

// Hash table
node *table[N];

// Returns true if word is in dictionary else false
bool check(const char *word)
{
   int i = hash(word);
   for (node *tmp = table[i]; tmp != NULL; tmp = tmp->next)//tmp refers to table[i] which Valgrind seems to think may be unitialized
   {
        if(strcasecmp(word, tmp->word) == 0)
            return true;
   }
    // TODO
    return false;
}

// Hashes word to a number
unsigned int hash(const char *word)
{
    // TODO
    int i = tolower(word[0]) - 97;
    return i;
}

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    //populate the table[i] array
    for (int i = 0; i < N; i++)
    {
        table[i] = NULL;
    }

    // Open the dictionary
    FILE *file = fopen(dictionary, "r");
    if (!file)
    {
        printf("null character\n");
        return 1;
    }

    //create place to store words temporarily
    char tmpword[LENGTH + 1];

    //Set wordcount to zero for Size
    wordcount = 0;

    //Start scanning the file
    while(fscanf(file, "%s", tmpword) != EOF)
    {
        node *n = malloc(sizeof(node));//AAAHHHHAHAHAHAHAHAHAHAAHHHAA!!!!!!!!!!!!!!!!!!!!!!!!!!
        if(n == NULL)
        {
            return 1;
        }
        strcpy(n->word, tmpword);
        wordcount++;

        // Hash the word for the correct table index
        int i = hash(tmpword);

        // insert word into hash table
        if (table[i] != NULL)
        {
            n->next = table[i];
            table[i] = n;
        }
        else
        // if it's the first word for that table index, set the index to that word
        {
            table[i] = n;
        }
    }
    fclose(file);
    return true;
}

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

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
    // TODO
    for (int i = 0; i < N; i++)
    {
        if (table[i] != NULL)
        {
            node* cursor = table[i];
            while (cursor != NULL) //cursor is also dependent on table[i]
            {
                node* temp = cursor;
                cursor = cursor->next;
                free(temp);
            }
        }
    }

    return true;
}
3
  • In case not clear: Line 31: for (node *tmp = table[i]; tmp != NULL; tmp = tmp->next)//tmp refers to table[i] which Valgrind seems to think may be unitialized /////\n Line 117: while (cursor != NULL) //cursor is also dependent on table[i]////\n Line 74: node *n = malloc(sizeof(node));//AAAHHHHAHAHAHAHAHAHAHAAHHHAA!!!!!!!!!!!!!!!!!!!!!!!!!! – cormacl Dec 19 '20 at 21:22
  • Remember each node contains a pointer too, which is not always initialized in the code. – DinoCoderSaurus Dec 20 '20 at 17:36
  • @DinoCoderSaurus Ahhh! Of course! Thanks a million! That solved it straight away! :) – cormacl Dec 20 '20 at 21:48
0

First off, thank you to DinoCoderSaurus for the tip! You're a lifesaver!

The problem was that when I malloc'd node *N in line 74 (which Valgrind was pointing to), I neglected to initialize the pointer part of that node, i.e. n->next. By setting that to NULL it solved my problem.

Here's how the correct code should look:

      node *n = malloc(sizeof(node));//AAAHHHHAHAHAHAHAHAHAHAAHHHAA!!!!!!!!!!!!!!!!!!!!!!!!!!
        if(n == NULL)
        {
            return 1;
        }
        strcpy(n->word, tmpword); //initializing n->word
        n->next = NULL; //initializing the pointer n is pointing to (n->next)
        wordcount++;

You must log in to answer this question.

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