Pset 5 - valgrind errror because of memory leak

Question

Succesfully finished Pset5 except for one memory leak error.

This is the Valgrind and Help50 report:

==32337== 
==32337== HEAP SUMMARY:
==32337==     in use at exit: 47 bytes in 1 blocks
==32337==   total heap usage: 224,954 allocs, 224,953 frees, 34,837,871 bytes allocated
==32337== 
==32337== 47 bytes in 1 blocks are definitely lost in loss record 1 of 1
==32337==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32337==    by 0x425760: load (dictionary.c:197)
==32337==    by 0x4209A2: main (speller.c:40)
==32337== 
==32337== LEAK SUMMARY:
==32337==    definitely lost: 47 bytes in 1 blocks
==32337==    indirectly lost: 0 bytes in 0 blocks
==32337==      possibly lost: 0 bytes in 0 blocks
==32337==    still reachable: 0 bytes in 0 blocks
==32337==         suppressed: 0 bytes in 0 blocks
==32337== 
==32337== For counts of detected and suppressed errors, rerun with: -v
==32337== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Helping with...                                                                                                                                                      

==32337== 47 bytes in 1 blocks are definitely lost in loss record 1 of 1
==32337==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32337==    by 0x425760: load (dictionary.c:197)
==32337==    by 0x4209A2: main (speller.c:40)

Looks like your program leaked 47 bytes of memory. Did you forget to free memory that you allocated via malloc? Take a closer look at line 197 of
dictionary.c.

I've loaded the dictionary as follows:

1. A hash table based on the length of each word (the 'length' struct)
2. Then a hash table based on the combination of the first two chars (if the word is at least two chars long. (the 'first2chars' struct).
3. Then a hash table based on the combination of the middle two chars (if the word is at least three chars long. (the 'middle2chars' struct)
4. Then each word is loaded into a linked-list node.

This is the code:

root = calloc(1, sizeof(length));
char *ptr = malloc(sizeof(char) * (LENGTH + 2));

for((ptr = fgets(ptr, 47, dictfile)); ptr != NULL; (ptr = fgets(ptr, 47, dictfile)))
{
                    int s = (strlen(ptr) - 1);

                    // Path to first node for word of 1 letter
                    if(s == 1)
                    {
                        if(root->lengthword_a == NULL)
                        {
                            nodell *n = calloc(1, sizeof(nodell));
                            root->lengthword_a = n;
                            travnode = n;
                        }

                        else if(root->lengthword_a != NULL)
                        {
                            travnode = root->lengthword_a;
                        }
                    }

                    // Path to first node for word of 2 letters
                    else if(s == 2)
                    {

                        // Hash word according to length of the word
                        if(root->lengthword_bc[s] == NULL)
                        {
                            first2chars *n = calloc(1, sizeof(first2chars));
                            root->lengthword_bc[s] = n;
                            travfirst = n;
                        }

                        else if(root->lengthword_bc[s] != NULL)
                        {
                            travfirst = root->lengthword_bc[s];
                        }

                        // Calculate hash code for first two letters and update if second letter is apostrophe
                        int c1 = (27 * (ptr[0] - 97)) + (1 * (ptr[1] - 97));

                        if(ptr[1] == '\'')
                        {
                            c1 = (27 * (ptr[0] - 97)) + 26;
                        }

                        // Check if combination of first letters is used before
                        if(travfirst->combi1_b[c1] == NULL)
                        {
                            nodell *n = calloc(1, sizeof(nodell));
                            travfirst->combi1_b[c1] = n;
                            travnode = n;
                        }

                        else if(travfirst->combi1_b[c1] != NULL)
                        {
                            travnode = travfirst->combi1_b[c1];
                        }
                    }

                    // Path to first node for word of 3 or more letters
                    else if(s > 2)
                    {

                        // Hash word according to length of the word
                        if(root->lengthword_bc[s] == NULL)
                        {
                            first2chars *n = calloc(1, sizeof(first2chars));
                            root->lengthword_bc[s] = n;
                            travfirst = n;
                        }

                        else if(root->lengthword_bc[s] != NULL)
                        {
                            travfirst = root->lengthword_bc[s];
                        }

                        // Calculate hash code for first two letters and update if second letter is apostrophe
                        int c1 = (27 * (ptr[0] - 97)) + (1 * (ptr[1] - 97));

                        if(ptr[1] == '\'')
                        {
                            c1 = (27 * (ptr[0] - 97)) + 26;
                        }

                        // Calculate the number of the middle char
                        int f = (s / 2);

                        // Calculate hash code for middle two letters and update if second or third letter is apostrophe
                        int c2 = (27 * (ptr[f] - 97)) + (1 * (ptr[f + 1] - 97));

                        if(ptr[f] == '\'')
                        {
                            c2 = (27 * 26) + (1 * (ptr[f + 1] - 97));
                        }

                        else if(ptr[f + 1] == '\'')
                        {
                            c2 = (27 * (ptr[f] - 97)) + 26;
                        }

                        // Check if combination of first letters is used before
                        if(travfirst->combi1_c[c1] == NULL)
                        {
                            middle2chars *n = calloc(1, sizeof(middle2chars));
                            travfirst->combi1_c[c1] = n;
                            travmiddle = n;
                        }

                        else if(travfirst->combi1_c[c1] != NULL)
                        {
                            travmiddle = travfirst->combi1_c[c1];
                        }

                        // Check if combination of middle two letters is used before
                        if(travmiddle->combi2_c[c2] == NULL)
                        {
                            nodell *n = calloc(1, sizeof(nodell));
                            travmiddle->combi2_c[c2] = n;
                            travnode = n;
                        }

                        else if(travmiddle->combi2_c[c2] != NULL)
                        {
                            travnode = travmiddle->combi2_c[c2];
                        }
                    }

                    // Go through the linked list with words until at the end of the list
                    while(travnode->next != NULL)
                    {
                        travnode = travnode->next;
                    }

                    // Add word to linkedllist and allocate memory for new word
                    memcpy(travnode->dictword, ptr, s);
                    words++;
                    nodell *n = calloc(1, sizeof(nodell));
                    travnode->next = n;
                }

                loaded = 1;
                fclose(dictfile);
                free(ptr);
                return true;
            }

The error is in the line where I declare the variable 'ptr'.

I do not understand why freeing this pointer (at the end of the load program) does not solve the issue.

Thanks for your help!

Answer 1

The problem lies in these two lines and the usage of fgets.

char *ptr = malloc(sizeof(char) * (LENGTH + 2));

for((ptr = fgets(ptr, 47, dictfile)); ptr != NULL; (ptr = fgets(ptr, 47, dictfile)))
{ ...

The first line allocates LENGTH + 2 bytes and stores the address in ptr.

THe second line, the for loop, then calls fgets(). The fgets() call reads 47 bytes max from dictfile, allocates storage, saves the data to that storage, and puts the address in ptr.

The problem is related to how the fgets call is written. When it reaches the end of the input file, EOF, the fgets call returns NULL. Since the return value is written to ptr, it will overwrite the string location previously allocated with the malloc and the memory address is lost forever, causing the memory leak. In this case, the free() call at the end is trying to free a null pointer.

The for loop setup needs modification. Maybe a different pointer to contain the return value(s) from the fgets() call(s)? That way, the address stored in ptr won't get overwritten with NULL at EOF.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)