I am teaching myself C. My goal is to make a C function that just walks a query string and splits on the ampersand and the equals sign. I am getting stuck on this error from Valgrind.
==5411== Invalid free() / delete / delete[] / realloc()
==5411== at 0x402AC38: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5411== by 0x804857C: main (leak.c:28)
==5411== Address 0x420a02a is 2 bytes inside a block of size 8 free'd
==5411== at 0x402AC38: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5411== by 0x804857C: main (leak.c:28)
==5411==
==5411==
==5411== HEAP SUMMARY:
==5411== in use at exit: 0 bytes in 0 blocks
==5411== total heap usage: 1 allocs, 2 frees, 8 bytes allocated
==5411==
==5411== All heap blocks were freed -- no leaks are possible
==5411==
==5411== For counts of detected and suppressed errors, rerun with: -v
==5411== ERROR SUMMARY: 20 errors from 9 contexts (suppressed: 0 from 0)
and the backtrace :
*** Error in `./leak': free(): invalid pointer: 0x08c1d00a ***
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x767c2)[0xb75f17c2]
/lib/i386-linux-gnu/libc.so.6(+0x77510)[0xb75f2510]
./leak[0x804857d]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0xb7594905]
./leak[0x8048421]
======= Memory map: ========
08048000-08049000 r-xp 00000000 08:05 262764 /home/danny/dev/c-qs-parser/leak
08049000-0804a000 r--p 00000000 08:05 262764 /home/danny/dev/c-qs-parser/leak
0804a000-0804b000 rw-p 00001000 08:05 262764 /home/danny/dev/c-qs-parser/leak
08c1d000-08c3e000 rw-p 00000000 00:00 0 [heap]
b757a000-b757b000 rw-p 00000000 00:00 0
b757b000-b7729000 r-xp 00000000 08:05 1312132 /lib/i386-linux-gnu/libc-2.17.so
b7729000-b772b000 r--p 001ae000 08:05 1312132 /lib/i386-linux-gnu/libc-2.17.so
b772b000-b772c000 rw-p 001b0000 08:05 1312132 /lib/i386-linux-gnu/libc-2.17.so
b772c000-b772f000 rw-p 00000000 00:00 0
b772f000-b774a000 r-xp 00000000 08:05 1312589 /lib/i386-linux-gnu/libgcc_s.so.1
b774a000-b774b000 r--p 0001a000 08:05 1312589 /lib/i386-linux-gnu/libgcc_s.so.1
b774b000-b774c000 rw-p 0001b000 08:05 1312589 /lib/i386-linux-gnu/libgcc_s.so.1
b774c000-b7750000 rw-p 00000000 00:00 0
b7750000-b7751000 r-xp 00000000 00:00 0 [vdso]
b7751000-b7771000 r-xp 00000000 08:05 1312116 /lib/i386-linux-gnu/ld-2.17.so
b7771000-b7772000 r--p 0001f000 08:05 1312116 /lib/i386-linux-gnu/ld-2.17.so
b7772000-b7773000 rw-p 00020000 08:05 1312116 /lib/i386-linux-gnu/ld-2.17.so
bfe93000-bfeb4000 rw-p 00000000 00:00 0 [stack]
Aborted (core dumped)
finally here is the code:
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
int main() {
//char p[] = "t=quote&k=id&v=10";
char p[] = "t=quote";
char* token;
char* tk;
char* s;
unsigned short int found;
s = strdup(p);
if (s != NULL) {
while ((token = strsep(&s, "&")) != NULL) {
found = 0;
printf("TOKEN: %s\n\n", token);
while ((tk = strsep(&token, "=")) != NULL) {
printf("TK: %s\n\n", tk);
free(tk);
}
free(token);
}
}
free(s);
return 0;
}
Thanks
You're attempting to free something that isn't a pointer to a "freeable" memory address. Just because something is an address doesn't mean that you need to or should free it.
There are two main types of memory you seem to be confusing - stack memory and heap memory.
Stack memory lives in the live span of the function. It's temporary space for things that shouldn't grow too big. When you call the function main, it sets aside some memory for your variables you've declared (p,token, and so on).
Heap memory lives from when you malloc it to when you free it. You can use much more heap memory than you can stack memory. You also need to keep track of it - it's not easy like stack memory!
You have a few errors:
You're trying to free memory that's not heap memory. Don't do that.
You're trying to free the inside of a block of memory. When you have in fact allocated a block of memory, you can only free it from the pointer returned by malloc. That is to say, only from the beginning of the block. You can't free a portion of the block from the inside.
For your bit of code here, you probably want to find a way to copy relevant portion of memory to somewhere else...say another block of memory you've set aside. Or you can modify the original string if you want (hint: char value 0 is the null terminator and tells functions like printf to stop reading the string).
EDIT: The malloc function does allocate heap memory*.
"9.9.1 The malloc and free Functions
The C standard library provides an explicit allocator known as the malloc package. Programs allocate blocks from the heap by calling the malloc function."
~Computer Systems : A Programmer's Perspective, 2nd Edition, Bryant & O'Hallaron, 2011
EDIT 2: * The C standard does not, in fact, specify anything about the heap or the stack. However, for anyone learning on a relevant desktop/laptop machine, the distinction is probably unnecessary and confusing if anything, especially if you're learning about how your program is stored and executed. When you find yourself working on something like an AVR microcontroller as H2CO3 has, it is definitely worthwhile to note all the differences, which from my own experience with embedded systems, extend well past memory allocation.
From where did you get the idea that you need to free(token) and free(tk)? You don't. strsep() doesn't allocate memory, it only returns pointers inside the original string. Of course, those are not pointers allocated by malloc() (or similar), so free()ing them is undefined behavior. You only need to free(s) when you are done with the entire string.
Also note that you don't need dynamic memory allocation at all in your example. You can avoid strdup() and free() altogether by simply writing char *s = p;.
You can't call free on the pointers returned from strsep. Those are not individually allocated strings, but just pointers into the string s that you've already allocated. When you're done with s altogether, you should free it, but you do not have to do that with the return values of strsep.
Source: Stackoverflow.com