0

The program seems fine to me (other than this, it runs as intended), when I run valgrind, it says:

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

MISSPELLED WORDS


WORDS MISSPELLED:     0
WORDS IN DICTIONARY:  143091
WORDS IN TEXT:        6
TIME IN load:         2.20
TIME IN check:        0.02
TIME IN size:         0.02
TIME IN unload:       0.01
TIME IN TOTAL:        2.25

==5101== 
==5101== HEAP SUMMARY:
==5101==     in use at exit: 7,501,960 bytes in 133,964 blocks
==5101==   total heap usage: 143,098 allocs, 9,134 frees, 8,023,504 bytes allocated
==5101== 
==5101== LEAK SUMMARY:
==5101==    definitely lost: 0 bytes in 0 blocks
==5101==    indirectly lost: 0 bytes in 0 blocks
==5101==      possibly lost: 0 bytes in 0 blocks
==5101==    still reachable: 7,501,960 bytes in 133,964 blocks
==5101==         suppressed: 0 bytes in 0 blocks
==5101== Reachable blocks (those to which a pointer was found) are not shown.
==5101== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5101== 
==5101== For counts of detected and suppressed errors, rerun with: -v
==5101== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

As I understand it, the

==5101== LEAK SUMMARY:
==5101==    definitely lost: 0 bytes in 0 blocks
==5101==    indirectly lost: 0 bytes in 0 blocks
==5101==      possibly lost: 0 bytes in 0 blocks
==5101==    still reachable: 7,501,960 bytes in 133,964 blocks
==5101==         suppressed: 0 bytes in 0 blocks

means that there's no memory leak. But when I run check50, it fails the final test (only) and says

:( program is free of memory errors
valgrind tests failed; rerun with --log for more information. 
Log 
running valgrind --show-leak-kinds=all --xml=yes --xml-file=/tmp/tmptipux_7m -- ./speller substring/dict substring/text... 
checking for output "MISSPELLED WORDS ca cats caterpill caterpillars WORDS MISSPELLED: 4 WORDS IN DICTIONARY: 2 WORDS IN TEXT: 6 "... 
checking that program exited with status 0... 
checking for valgrind errors... 
Conditional jump or move depends on uninitialised value(s): (file: dictionary.c, line: 109) 
Conditional jump or move depends on uninitialised value(s): (file: dictionary.c, line: 90) 
112 bytes in 2 blocks are still reachable in loss record 1 of 1: (file: dictionary.c, line: 54) 

My entire dictionary.c code:

// Implements a dictionary's functionality

#include <ctype.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "dictionary.h"

// Represents number of buckets in a hash table
#define N 26

// Represents a node in a hash table
typedef struct node
{
    char word[LENGTH + 1];
    struct node *next;
}
node;

// Represents a hash table
node *hashtable[N];

// Hashes word to a number between 0 and 25, inclusive, based on its first letter
unsigned int hash(const char *word)
{
    return tolower(word[0]) - 'a';
}

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

    // Open dictionary
    FILE *dicptr = fopen(dictionary, "r");
    if (dicptr == NULL)
    {
        unload();
        return false;
    }

    // Buffer for a word
    char worder[LENGTH + 1];

    // Insert words into hash table
    while(true)
    {
        node* newword = malloc(sizeof(node));
        if(fscanf(dicptr, "%s", worder) == EOF)
        {
            free(newword);
            break;
        }
        int firstLetter = hash(worder);
        for(int j = 0, lengther = strlen(worder); j < lengther; j++)
        {
            newword -> word [j] = worder[j];
        }
        newword -> word [strlen(worder)] = '\0';
        if(hashtable[firstLetter] != NULL)
        {
            newword -> next = hashtable[firstLetter];
            hashtable[firstLetter] = newword;
        }else{
            hashtable[firstLetter] = newword;
            newword -> next = NULL;
        }
    }

    // Close dictionary
    fclose(dicptr);

    // Indicate success
    return true;
}

// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
    unsigned int numWords = 0;

        for(int i = 0; i < N; i++)
        {
            node *inptr = hashtable[i];
            while(inptr != NULL)
            {
                inptr = inptr->next;
                numWords++;
            }
        }
    return numWords;
}

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    char worder[LENGTH + 1];
    for(int i = 0, lengtheroni = strlen(word); i<lengtheroni; i++)
    {
        worder[i] = tolower(word[i]);
    }
    worder[strlen(word)] = '\0';
    node* inptr = hashtable[hash(worder)];
    while(inptr != NULL)
    {
        if(strcmp(worder, inptr->word) == 0)
            return true;
        else
            inptr = inptr->next;
    }
    return false;
}

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
    for(int i = 0; i < N; i++)
    {
        node *crtptr = hashtable[i];
        while(crtptr != NULL)
        {
            node *tempo = crtptr;
            crtptr = crtptr->next;
            free(tempo);
        }
        free(crtptr);
        return true;
    }
    return false;
}

What is the issue with this? I think I've freed everything.

Any help would be appreciated!

1

Move the return true; out of your for loop in unload. You unload all the words starting with "a", but no other words, since you return from the function early.

Unrelated, your unload has a free(crtptr); that can only be called when crtptr is NULL. Is harm- but also useless.

4
  • How do you implement the fact that it's supposed to return false if unsuccessful? Either way, I changed unload to: for(int i = 0; i < N; i++) { node *crtptr = hashtable[i]; while(crtptr != NULL) { node *tempo = crtptr; crtptr = crtptr->next; free(tempo); } } return true; However, even after doing this, it says there are 32 bytes in 1 block in use at exit. Edit:I'm not sure how to write the code code-like in a comment, so I just left this. – Amit Rajaraman Jul 31 '19 at 13:36
  • What would an unsuccessful unload look like, what should cause it? Just a return true; at the end should work. The 32 bytes are a bug introduced earlier (see cs50.stackexchange.com/questions/32984/…). And right, you can't write code blocks in comments, only inline code. You also cannot use regular links in comments (with text and URL different), and no similarly linked images, most features are missing in this context. – Blauelf Jul 31 '19 at 14:18
  • I'm not sure, the problem statement just says that // Unloads dictionary from memory, returning true if successful else false. If the 32 bytes is just a bug, then great, thanks! – Amit Rajaraman Jul 31 '19 at 14:21
  • Yeah, that statement says that. I couldn't think of a way unloading could fail and you be able to detect it. If unloading fails, it's most likely because some other part misbehaved and your data structure cannot be trusted. But you have no other source than your data structure, so you would never be able to detect it, and run into segfaults or whatever, unable to ever return false;. The best you could do would be to count the words, and check that you unload the same number of words as you load before. But unless you really messed up, those should be same. – Blauelf Jul 31 '19 at 16:02

You must log in to answer this question.

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