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 from integers (using strcat) and writes it into a file. Unfortunately it does have a memory leak. While tracking that leak down I simplified my code to the following. I can't seem to locate or even fix it. 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";

String literals are '\0' terminated automatically, you don't need to add it.

 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);

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

      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; it's an invalid free().

Answer 2

Your code is broken. The first pass through, you set oldmemory to output where output points to memory that wasn't allocated on the heap. Later, you attempt to free this memory. This generates the valgrind error about freeing memory that wasn't allocated via malloc. Hence, the original memory you allocated never does get freed.

Answer 3

Your app is crashing trying to free( "\0" ). (Just a note, if you want an empty string, "" is enough, "\0" is actually the string \0\0.

Instead of using malloc and strcpy, look at realloc, it does everything you want but better :) But you'd most likely want to build your string forward (counter = 0; counter < 5; count++) instead of going backwards

Answer 4

If you want to use this algorithm, the the initial space for what output points to should be allocated with malloc, thusly:

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

String literals are not allocated with malloc, and consequently can not be free'ed, which is the source of your problem.