realloc() leaking memory

Question

I have a function which adds a character to a string:

void AddChToString(char **str,char ch){
    int len=(*str)?strlen(*str):0;
    (*str)=realloc(*str, len+2);
    (*str)[len]=ch;
    (*str)[len+1]='\0';
}

Instruments (on the mac) and Valgrind are indicating that the line: (*str)=realloc(*str, len+2) is leaking memory. Is this an implementation issue with realloc? Or am I using it improperly?

Here is the output from Valgrind:

==39230== 6 bytes in 1 blocks are definitely lost in loss record 1 of 7
==39230==    at 0x100018B2D: realloc (vg_replace_malloc.c:525)
==39230==    by 0x100002259: AddChToString (in ./OpenOtter)
==39230==    by 0x10000477B: QueryMapFromString (in ./OpenOtter)
==39230==    by 0x100684CD2: ???
==39230==    by 0x100001FB0: RequestHandler (in ./OpenOtter)
==39230==    by 0x100065535: _pthread_start (in /usr/lib/libSystem.B.dylib)
==39230==    by 0x1000653E8: thread_start (in /usr/lib/libSystem.B.dylib)
==39230== 
==39230== 9 bytes in 1 blocks are definitely lost in loss record 2 of 7
==39230==    at 0x100018B2D: realloc (vg_replace_malloc.c:525)
==39230==    by 0x100002259: AddChToString (in ./OpenOtter)
==39230==    by 0x10000298E: ParseHTTPRequest (in ./OpenOtter)
==39230==    by 0x100004151: OpenRoutesFile (in ./OpenOtter)
==39230==    by 0x10000142B: main (in ./OpenOtter)
==39230== 
==39230== 45 bytes in 5 blocks are definitely lost in loss record 3 of 7
==39230==    at 0x100018B2D: realloc (vg_replace_malloc.c:525)
==39230==    by 0x100002259: AddChToString (in ./OpenOtter)
==39230==    by 0x10000298E: ParseHTTPRequest (in ./OpenOtter)
==39230==    by 0x100001EB4: RequestHandler (in ./OpenOtter)
==39230==    by 0x100065535: _pthread_start (in /usr/lib/libSystem.B.dylib)
==39230==    by 0x1000653E8: thread_start (in /usr/lib/libSystem.B.dylib)
==39230== 
==39230== LEAK SUMMARY:
==39230==    definitely lost: 60 bytes in 7 blocks
==39230==    indirectly lost: 0 bytes in 0 blocks
==39230==      possibly lost: 0 bytes in 0 blocks
==39230==    still reachable: 1,440 bytes in 4 blocks
==39230==         suppressed: 0 bytes in 0 blocks

Thanks.

Answer 1

Call to realloc itself does not leak memory. You should make sure that the memory for reallocated string is released with free after it's not needed any more.

Answer 2

Is your instrumentation indicating that there's an actual leak or that there's a potential leak?

Using realloc() like you are will leak memory if realloc() fails. In that case, it'll return NULL but will not free the original block. So you will have lost the pointer to the block and cannot free it (unless the pointer is stored elsewhere).

But this should be a rare occurrence (ie., when you've exhausted memory).

If that's what your tools are complaining about, you should be able to fix the leak warning with something like:

void AddChToString(char **str,char ch){
    int len=(*str)?strlen(*str):0;
    char* tmp = realloc(*str, len+2);

    if (!tmp) {
        // whatever error handling is appropriate
    }

    (*str)=tmp;
    (*str)[len]=ch;
    (*str)[len+1]='\0';
}

Answer 3

try reallocating using a separate variable, then calling strcpy to get the str variable into that space, like this:

void AddChToString(char **str,char ch){
char *blah;
int len=(*str)?strlen(*str):0;
blah=realloc(NULL, len+2);
strcpy(blah, str); 
(*str)[len]=ch;   
(*str)[len+1]='\0'; } 

Answer 4

I don't know about implementation issues about realloc, but there is surely an opportunity for memory leak in this code: from the realloc manpage:

If realloc() fails the original block is left untouched; it is not freed or moved.

and, since realloc returns NULL when it fails, if it fails you lose your only pointer to the already allocated memory block, so you have a memory leak.

To avoid the problem, you should do:

char * temp=realloc(*str, len+2);
if(temp==NULL)
{
    /* handle the error in some way */
}
else
    *str=temp;