0

Error from valgrind

$ valgrind ./speller texts/cat.txt
==1453== Memcheck, a memory error detector
==1453== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1453== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==1453== Command: ./speller texts/cat.txt
==1453== 

MISSPELLED WORDS

==1453== Invalid read of size 1
==1453==    at 0x4C33614: strcasecmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1453==    by 0x40104F: check (dictionary.c:41)
==1453==    by 0x400C49: main (speller.c:112)
==1453==  Address 0x55cb2b0 is 0 bytes inside a block of size 56 free'd
==1453==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1453==    by 0x401238: load (dictionary.c:124)
==1453==    by 0x400924: main (speller.c:40)
==1453==  Block was alloc'd at
==1453==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1453==    by 0x401147: load (dictionary.c:87)
==1453==    by 0x400924: main (speller.c:40)
==1453== 
==1453== Invalid read of size 8
==1453==    at 0x401066: check (dictionary.c:48)
==1453==    by 0x400C49: main (speller.c:112)
==1453==  Address 0x55cb2e0 is 48 bytes inside a block of size 56 free'd
==1453==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1453==    by 0x401238: load (dictionary.c:124)
==1453==    by 0x400924: main (speller.c:40)
==1453==  Block was alloc'd at
==1453==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1453==    by 0x401147: load (dictionary.c:87)
==1453==    by 0x400924: main (speller.c:40)
==1453== 
==1453== 
==1453== More than 10000000 total errors detected.  I'm not reporting any more.
==1453== Final error counts will be inaccurate.  Go fix your program!
==1453== Rerun with --error-limit=no to disable this cutoff.  Note
==1453== that errors may occur in your program without prior warning from
==1453== Valgrind, because errors are no longer being displayed.
==1453== 

Code is here

// Implements a dictionary's functionality

#include <stdbool.h>
#include <string.h>
#include <strings.h>
#include <stdlib.h>
#include <stdio.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
const unsigned int N = 26;

unsigned int count = 0;

// Hash table
node *table[N];

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    // TODO
    int alpha = hash(word);

    //cursor for checking words
    node *cursor = table[alpha];

    //keep running till cursor points at null
    while (cursor != NULL)
    {
        //compare word with dictionary words, case insensitive
        if (strcasecmp(cursor->word, word) == 0)
        {
            return true;
        }
        //cursor points at next word
        else
        {
          cursor = cursor->next;
        }
    }

    return false;
}

// Hashes word to a number
unsigned int hash(const char *word)
{
    // Assigning words in alphanumeric order where a = 0 z = 25 case insensitive
    if (isupper(word[0]))
    {
        //uppercase ASCII starts at 65
        return (word[0] - 65);
    }
    else
    {
        //lowercase at 97
        return (word[0] - 97);
    }
}

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    //Read dictionary to dic
    FILE *dic = fopen(dictionary, "r");

    if (dic == NULL)
    {
        printf("Invalid file \n");

        return false;
    }

    char dic_word [LENGTH + 1];

    //LIST: a temporary node to load in table
    node *list = malloc(sizeof(node));

    if (list == NULL)
        {
            printf("UNABLE TO CREATE LIST \n");
            return false;
        }

    //scan strings from dic to dic_word
    while (fscanf(dic, "%s", dic_word) != EOF)
    {
        //copy string to word branch of list node
        strcpy(list->word, dic_word);

        //taking word count for size function
        count++;

        //add hash figure to word in dic_word
        int alpha = hash(dic_word);

        //if first object under the alphabet, create this new list
        if (table[alpha] == NULL)
        {
            table[alpha] = list;

            list->next = NULL;
        }

        //if an object already exist in this list, continue adding
        else
        {
            list->next = table[alpha];

            table[alpha] = list;
        }
    }
    //free list after malloc
    free(list);

    return true;
}

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

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
    //For N tables created
    for (int i = 0; i < N; i++)
    {
        //point cursor at beginning of list
        node *cursor = table[i];

        while (cursor != NULL)
        {
            //point tmp to cursor
            node *tmp = cursor;

            //point cursor to next element
            cursor = cursor->next;

            //free what tmp holds
            free (tmp);
        }
    }
    return true;
}
  • What are you asking? What is it that you don't understand? – Cliff B Aug 9 at 19:22
  • I cant interpret the valgrind report – Olhuwhorlhey Aug 15 at 18:19
  • check 50 also reports errors, I think understanding valgrind can help me fix it – Olhuwhorlhey Aug 15 at 18:37
1

You don't need to allocate memory to go checking through a node in your check function. You only need a pointer.

Let's go through your 'check' function:

*cursor = malloc(sizeof(node));

Here you're doing two things: you're allocating memory of the size of a node and creating a pointer 'cursor' pointing to that memory.

cursor = table[alpha];

Here you make the pointer 'cursor' point to the first node of 'table[alpha]'. The problem is that in this point you're losing the access to the memory you allocated in the previous sentence, as 'cursor' is not pointing at it anymore, so you won't be able to free it (as valgrind is reporting).

if (strcasecmp(cursor->word, word) == 0)
    {
        free(cursor);

Here 'cursor' is pointing at the node containing the correct word. By freing it, the node won't be reachable anymore, and probably there will be problems when it tries to check another word in the same table.

| improve this answer | |
  • Thank you, I've made the necessary corrections. Though valgrind and check 50 still gives the same error. – Olhuwhorlhey Aug 15 at 18:26
  • Sorry that your corrections didn't work. When I looked to It, It did pass check50 with my corrections. – Tritum Aug 16 at 10:46
  • Did you use the new code on the edited post or you corrected the previous code on the original post? – Olhuwhorlhey Aug 16 at 11:42
  • In my answer I was pointing to the errors in the Check function. With the changes you've made in the Load function, your code doesn't work and not only is failling valgrind, but all the points of check50. – Tritum Aug 16 at 12:29

You must log in to answer this question.

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