0

56 Byte memory leak - not sure where it is coming from. Valgrind points to the malloc'd space for nodnew in the load function - in my first version, I created the pointer nodenew and malloc'd the space inside the while loop - after I found the leak, I thought it was maybe because it would malloc one additional time when it was not needed (and thus not attached to the hashtable and unloaded). So I moved the pointer declaration outside of the while loop so that I could free it at the end of load(). Didn't change a thing.

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    //open the  dictonary file and checks for an error
    FILE *inptr = fopen(dictionary, "r");
    if (inptr == NULL)
    {
        return false;
    }
    //go through the whole file
    char *buffer = malloc(sizeof(char) * (LENGTH + 1));
    unsigned long temphash;
    node *nodenew = NULL;
    while (fscanf(inptr, "%s", buffer) != EOF)
    {
        wordcount++;
        //hash the word to find the right bucket
        temphash = hash(buffer);
        //put the word into a node
        nodenew = malloc(sizeof(node));
        if (nodenew == NULL)
        {
            printf("Could not malloc node.\n");
            return false;
        }
        //nodenew->word = buffer;
        strcpy(nodenew->word, buffer);
        nodenew->next = NULL;

        //check to see if there is a collision
        if (hashtable[temphash] == NULL)
        {
            hashtable[temphash] = nodenew;
        }
        else //collision, insert node at start of list
        {
            nodenew->next = hashtable[temphash];
            hashtable[temphash] = nodenew;
        }
    }
    fclose(inptr);
    free(buffer);
    free(nodenew);
    loaded = true;
    return true;
}

And here's what Valgrind spits out:

==13779== 
==13779== HEAP SUMMARY:
==13779==     in use at exit: 56 bytes in 1 blocks
==13779==   total heap usage: 160,850 allocs, 160,850 frees, 8,831,054 bytes allocated
==13779== 
==13779== 56 bytes in 1 blocks are still reachable in loss record 1 of 1
==13779==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13779==    by 0x4228B6: load (dictionary.c:80)
==13779==    by 0x420962: main (speller.c:40)
==13779== 
==13779== LEAK SUMMARY:
==13779==    definitely lost: 0 bytes in 0 blocks
==13779==    indirectly lost: 0 bytes in 0 blocks
==13779==      possibly lost: 0 bytes in 0 blocks
==13779==    still reachable: 56 bytes in 1 blocks
==13779==         suppressed: 0 bytes in 0 blocks
==13779== 
==13779== For counts of detected and suppressed errors, rerun with: -v
==13779== ERROR SUMMARY: 561606 errors from 4 contexts (suppressed: 0 from 0)

Line 80 of dictionary.c is: nodenew = malloc(sizeof(node));

UPDATE: I ended up finding the solution in the unload() function. It was a dumb little typo in a loop, oh well. There's two more issues I'm having with valgrind though.

==1599== Conditional jump or move depends on uninitialised value(s)
==1599==    at 0x59ACE1F: tolower (ctype.c:46)
==1599==    by 0x4223C7: check (dictionary.c:39)
==1599==    by 0x421333: main (speller.c:112)
==1599==  Uninitialised value was created by a stack allocation
==1599==    at 0x420834: main (speller.c:21)
==1599== 
==1599== Use of uninitialised value of size 8
==1599==    at 0x59ACE33: tolower (ctype.c:46)
==1599==    by 0x4223C7: check (dictionary.c:39)
==1599==    by 0x421333: main (speller.c:112)
==1599==  Uninitialised value was created by a stack allocation
==1599==    at 0x420834: main (speller.c:21)
==1599== 

** UPDATE 2: ** Figured it all out, thanks again all who helped!

  • Could you maybe add your unload function (as an edit to the question)? – Blauelf Nov 12 '18 at 8:41
  • The point is that this is the place where the memory was allocated, while we don't know where it failed to be unloaded. – Blauelf Nov 12 '18 at 9:54
  • Oh, and the declaration of your node type and your hash table including its length might be a nice bonus, depending on what's wrong. – Blauelf Nov 12 '18 at 10:39
1

In load, remove free(nodenew);. Completely, as requested by accepted answer https://cs50.stackexchange.com/a/30559/12099. The node is still in use.

If that doesn't solve the issue, add more data to your question and I'll edit my answer.

  • Hey! Thank you for getting back to me - I removed the free(newnode); line, and still have a memory leak somewhere. After I post this comment, I'll be updating the original post with the rest of the code. – stygarfield Nov 13 '18 at 4:32

You must log in to answer this question.

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