realloc and free causes "double free or corruption"

Question

Bear with me. I have not coded in c in 8 years and am totally baffled why my string manipulation is not working. I am writing a program that loops forever. In the loop I initialize two char pointers each is passed to a function that add text to the char pointer (array). When the functions are done I print the char pointer and free the two char pointers. However the program dies after 7 iterations with the following error message

* glibc detected * ./test: double free or corruption (fasttop): 0x0804a168 ***

#include sys/types.h
#include sys/stat.h
#include fcntl.h
#include string.h
#include stdio.h
#include stdlib.h
#include errno.h
#include time.h

char *SEPERATOR = "|";

void getEvent (char* results);
void getTimeStamp(char* timeStamp, int timeStampSize);
void stringAppend(char* str1, char* str2);

int main (int argc, char *argv[])
{
  int i = 0; 
  while(1)
  { 
    i++;
    printf("%i", i);    

    char* events= realloc(NULL, 1); 
    events[0] = '\0';
    getEvent(events);

    char* timestamp= realloc(NULL, 20);
    timestamp[0] = '\0';
    getTimeStamp(timestamp, 20);

    printf("%s", events);
    printf("timestamp: %s\n", timestamp);

    free(events);
    free(timestamp);
  } 
}

void getEvent (char* results)
{
  stringAppend(results, "a111111111111");
  stringAppend(results, "b2222222222222");
}

void getTimeStamp(char* timeStamp, int timeStampSize)
{
  struct tm *ptr;
  time_t lt;
  lt = time(NULL);
  ptr = localtime(&lt);
  int r = strftime(timeStamp, timeStampSize, "%Y-%m-%d %H:%M:%S", ptr);
}

void stringAppend(char* str1, char* str2)
{   
  int arrayLength = strlen(str1) + strlen(str2) + strlen(SEPERATOR) + 1;
  printf("--%i--",arrayLength);

  str1 = realloc(str1, arrayLength);
  if (str1 != NULL)
  {
    strcat(str1, SEPERATOR);
    strcat(str1, str2);
  }
  else
  {
    printf("UNABLE TO ALLOCATE MEMORY\n");
  }
}

Answer 1

You are reallocating str1 but not passing the value out of your function, so the potentially changed pointer is leaked, and the old value, which has been freed by realloc, is freed again by you. This causes the "double free" warning.

Answer 2

The problem is that while stringAppend reallocates the pointers, only stringAppend is aware of this fact. You need to modify stringAppend to take pointer-to-pointers (char **) so that the original pointers are updated.

Answer 3

This line in stringAppend:

str1 = realloc(str1, arrayLength);

changes the value of a local variable in stringAppend. This local variable named str1 now points to either the reallocated memory or NULL.

Meanwhile local variables in getEvent keep the values they had before, which now usually point to freed memory.

Answer 4

All the comments where very helpfull. Of course it makes total sense why the error was happening. I ended up solving it by making the following changes.

For both the getEvent and stringAppend I return the char pointer.

e.g.

char* stringAppend(char* str1, char* str2) 
{    
  int arrayLength = strlen(str1) + strlen(str2) + strlen(SEPERATOR) + 1; 
  printf("--%i--",arrayLength); 

  str1 = realloc(str1, arrayLength); 
  if (str1 != NULL) 
  { 
    strcat(str1, SEPERATOR); 
    strcat(str1, str2); 
  } 
  else 
  { 
    printf("UNABLE TO ALLOCATE MEMORY\n"); 
  } 
  return str1;
} 

Answer 5

This isn't an answer to your question (and you don't need one, since the error has been pointed out), but I do have some other comments about your code:

char* events= realloc(NULL, 1); 
events[0] = '\0';

You don't test that realloc successfully allocated memory.

char* timestamp= realloc(NULL, 20);
timestamp[0] = '\0';

Same problem here. In this case, you don't need realloc at all. Since this is a fixed-size buffer, you could use just:

char timestamp[20] = "";

And don't do this:

str1 = realloc(str1, arrayLength);

because if realloc fails, you'll orphan the memory that str1 was pointing to before. Instead:

char* temp = realloc(str1, arrayLength);
if (temp != NULL)
{
    str1 = temp;
    ...
}

Note that since you're modifying stringAppend to return the new string, you should do similar checks in the calling functions.

Also, "separator" is spelled with two As, not with two Es.