Invalid read of size 1 when using valgrind

Question

I am getting this output when using valgrind:

==19923== Invalid read of size 1
==19923==    at 0x52CCCC0: vfprintf (vfprintf.c:1632)
==19923==    by 0x52F4772: vasprintf (vasprintf.c:59)
==19923==    by 0x52D3A56: asprintf (asprintf.c:35)
==19923==    by 0x400D77: addToList (pa1.c:124)
==19923==    by 0x4010AF: md5Hash (pa1.c:220)
==19923==    by 0x401138: newFile (pa1.c:244)
==19923==    by 0x401260: searchDirects (pa1.c:280)
==19923==    by 0x401451: main (pa1.c:339)
==19923==  Address 0x585b720 is 0 bytes inside a block of size 27 free'd
==19923==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19923==    by 0x401144: newFile (pa1.c:246)
==19923==    by 0x401260: searchDirects (pa1.c:280)
==19923==    by 0x401382: directoryCheck (pa1.c:312)
==19923==    by 0x4012CD: searchDirects (pa1.c:290)
==19923==    by 0x401451: main (pa1.c:339)
==19923==  Block was alloc'd at
==19923==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19923==    by 0x52F47D7: vasprintf (vasprintf.c:73)
==19923==    by 0x52D3A56: asprintf (asprintf.c:35)
==19923==    by 0x40112C: newFile (pa1.c:239)
==19923==    by 0x401260: searchDirects (pa1.c:280)
==19923==    by 0x401382: directoryCheck (pa1.c:312)
==19923==    by 0x4012CD: searchDirects (pa1.c:290)
==19923==    by 0x401451: main (pa1.c:339)

Here is where I have narrowed it down to being the problem in my code:

else
{    
    currentList = headList;
    printf("This is where I will put the code to create the Linked list!\n");
    if(currentList != 0)
    {
        while(currentList->nextList != 0)
        {
            if(currentList->key = key)
            {
                asprintf(&buff, "%s %s", currentList->value, dirValue); // PROBLEM IS HERE
                printf("Here is the new string that holds the directory paths: %s\n", buff);
                currentList->value = buff;
                free(buff);
                printf("Here is the new string that holds the directory paths: %s\n", currentList->value);
                return;
            }
            currentList = currentList->nextList;
        }

        currentList->nextList = malloc(sizeof(struct List));
        printf("Adding a new node\n");
        //Reached the end of the Linked list and didn't find the
        //same key so create a new node.
        currentList = currentList->nextList;
        currentList->key = key;
        currentList->nextList = NULL;
        currentList->value = dirValue;
    }
}

I have many errors, and providing the entire valgrind output would be overkill I feel. All of the errors I am getting come back to where I am using asprintf() ("addToList (pa1.c:124)" line in the report). Above the code I initialize char *buff = NULL;, and currentList->value holds a string that I am wanting to append onto using asprintf. Any guesses to why I am getting the numerous Invalid read size 1 errors would be greatly appreciated. Also, the string is appended as expected when I do print out the result from asprintf() Thanks.

Not sure whether it causes the exact error but `if(currentList->key = key)` needs to be `if(currentList->key == key)` — kaylum, Jan 16 '17 at 05:42
`currentList->value = buff; free(buff);` ? Deallocating something you are linked to seems to be a serious logic error at least. — Jean-Baptiste Yunès, Jan 16 '17 at 05:50
`currentList->value = buff; free(buff);` makes no sense. Why assign a pointer you're about to free? — melpomene, Jan 16 '17 at 05:50
@kaylum - Yep, you are absolutely correct. Thanks, for spotting that! — MuffinMan1042, Jan 17 '17 at 04:22
@Jean-Baptiste Yunes - Haha, you and melpomene both made me realize the mistake I was doing. Thanks! — MuffinMan1042, Jan 17 '17 at 04:24
@melpomene - Please see my above comment. However, you are correct and I appreciate the input. — MuffinMan1042, Jan 17 '17 at 04:24

score 2 · Accepted Answer · answered Jan 16 '17 at 05:51

The invalid read is at the start of an already freed block of memory; you are trying to use memory after you've freed it.

currentList->value = buff;
free(buff);
printf("Here is the new string that holds the directory paths: %s\n", currentList->value);
return;

You use currentList->value = buff; in the printf(), but you've just freed buff. This spells an impending disaster. If currentList->value is going to be used in calling functions after the return, you've got major problems.

You can fix the one warning by moving the printf() before the free(), but the code looks like you're going to have major issues. It might be better to delete the free(buff); — though you'd have to work out where the data is actually freed instead.

Although both the users in the above comments pointed out my flaw, I am marking your answer correct for clarity. I removed the free because I actually need the new string. Silly mistake, but another explanation of what I was doing made it quite clear what my issue was (the calling of free'd memory as you mentioned above). Thanks! — MuffinMan1042, Jan 17 '17 at 04:27

Invalid read of size 1 when using valgrind

1 Answers1