Where is the memory leak in this code and how to fix it?

Question

In my project, I have a method that creates a string of integers (using strcat) and writes it to a file. Unfortunately, he has a memory leak. By tracking this leak, I simplified my code as follows. I can't seem to find or even fix this. This is the code:

#include <stdio.h> #include <stdlib.h> #include <string.h> int main(int argc, char* argv[] ) { char* output = "\0"; int counter = 5; while(counter > 0) { char buffer[20]; sprintf(buffer, "%u", counter); char* temp; temp = malloc((strlen(output) + strlen(buffer) + 1)); strcpy(temp, buffer); strcat(temp, output); char* oldmemory = output; output = temp; free(oldmemory); counter--; } printf("output: %s\n", output); free(output); return 0; } 

Valgrind returns:

 ==7125== Memcheck, a memory error detector ==7125== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al. ==7125== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info ==7125== Command: ./foo ==7125== Parent PID: 4455 ==7125== ==7125== Invalid free() / delete / delete[] ==7125== at 0x4024B3A: free (vg_replace_malloc.c:366) ==7125== by 0x8048662: main (foo.c:20) ==7125== Address 0x8048780 is not stack'd, malloc'd or (recently) free'd ==7125== ==7125== ==7125== HEAP SUMMARY: ==7125== in use at exit: 0 bytes in 0 blocks ==7125== total heap usage: 5 allocs, 6 frees, 20 bytes allocated ==7125== ==7125== All heap blocks were freed -- no leaks are possible ==7125== ==7125== For counts of detected and suppressed errors, rerun with: -v ==7125== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 15 from 8) 

Where is the memory leak and how can I fix it?

Answer 1

 #include <stdio.h> #include <stdlib.h> int main(int argc, char* argv[] ) { char* output = "\0"; 

The string literals "\ 0" end automatically, you do not need to add them.

  int counter = 5; while(counter > 0) { char buffer[20]; sprintf(buffer, "%u", counter); char* temp; temp = malloc((strlen(output) + strlen(buffer) + 1)); strcpy(temp, buffer); strcat(temp, output); char* oldmemory = output; output = temp; free(oldmemory); 

This free() is called for the first time, freeing the initial value of the output, which is a pointer to the string literal "\0" . Calling free() for anything other than a valid pointer returned from *alloc() or NULL is undefined.

  counter--; } printf("output: %s\n", output); free(output); return 0; } 

valgrind reports:

 ==7125== Invalid free() / delete / delete[] ==7125== at 0x4024B3A: free (vg_replace_malloc.c:366) ==7125== by 0x8048662: main (foo.c:20) ==7125== Address 0x8048780 is not stack'd, malloc'd or (recently) free'd 

This is not a memory leak; this is not true free() .

Answer 2

Your code does not work. First go through, you set oldmemory to output, where the output points point to memory that was not allocated on the heap. Later you try to free this memory. This generates a valgrind error about freeing memory that was not allocated via malloc. Therefore, the original memory you allocated is never freed.

Answer 3

Your application crashes trying to free ("\ 0"). (Just a note, if you need an empty string, "" is enough, "\ 0" is actually the string \ 0 \ 0.

Instead of using malloc and strcpy, look at realloc, it will do everything you need, but better :) But you most likely want to build your line ahead (counter = 0; counter <5; count ++) instead of to go back

Answer 4

If you want to use this algorithm, the initial space for the output points must be allocated using malloc:

  char *output = malloc(1); if(!output) { /* handle error */ } output[0] = '\0'; ... rest of code as is ... 

String literals are not allocated by malloc and therefore cannot be free 'ed, which is the source of your problem.