Mona Mona - 3 days ago 5
C Question

Memory leak after using calloc and free?

I tested my software with "valgrind --leak-check=full", and it shows:

==90862== 7,627 bytes in 4 blocks are definitely lost in loss record 858 of 897
==90862== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==90862== by 0xD64991C: concat(int, ...) (Client.cpp:150)


I can't understand why, because I use free() after calloc.
Here's my code:

char* p = concat(2, buffOld, buff);
char *x;
while(true) {
x = p;
p = strstr(p,"\\final\\");
if(p == NULL) { break; }
*p = 0;
p+=7;
parseIncoming((char *)x,strlen(x));
}
free(p);


And the "concat" function:

char* concat(int count, ...)
{
va_list ap;
int i;

// Find required length to store merged string
int len = 1; // room for NULL
va_start(ap, count);
for(i=0 ; i<count ; i++)
len += strlen(va_arg(ap, char*));
va_end(ap);

// Allocate memory to concat strings
char *merged = (char*)calloc(sizeof(char),len);
int null_pos = 0;

// Actually concatenate strings
va_start(ap, count);
for(i=0 ; i<count ; i++)
{
char *s = va_arg(ap, char*);
strcpy(merged+null_pos, s);
null_pos += strlen(s);
}
va_end(ap);

return merged;
}


What I do wrong?

Answer

I can't understand why, because I use free() after calloc

Yes, but (if I understand correctly) you free() the wrong pointer.

You should copy p in another pointer (before modifing it) and free() the save copy.

Look at your code

char* p = concat(2, buffOld, buff);
char *x;
    while(true) {
        x = p;
        p = strstr(p,"\\final\\");
        if(p == NULL) { break; }
        *p = 0;
        p+=7;
        parseIncoming((char *)x,strlen(x));
    }
free(p);

The pointer p is initialized with the calloc-ed pointer but the while cicle modify it and return only when p is NULL.

So, when you call

free(p)

you're calling

free(nullptr)

--- EDIT ---

I still don't understand it. I added free(x) at the end, and it crashes

My initial suggestion to free(x) was a mistake of mine because I didn't pointed the attention to the fact that x is initializes with the p value but is modified in the while loop. Thanks again to Johnny Mopp for pointing my attention to it.

I suggest the use of another variable to memorize the original value of p (the exact value returned by calloc()) and free this value.

Something like

char* p = concat(2, buffOld, buff);
char *x;
char * const  forFree = p; /* <--- saving calloc() returned value */

while(true) {
    x = p;
    p = strstr(p,"\\final\\");
    if(p == NULL) { break; }
    *p = 0;
    p+=7;
    parseIncoming((char *)x,strlen(x));
}

free(forFree);
Comments