0

I have a function to append a string to another string:

char* strjoin(char* str1,const char* str2) {
  long len1 = strlen(str1);
  long len2 = strlen(str2);
  char* result = (char*)malloc(len1+len2+1);

  memcpy(result,str1,len1+1);
  memcpy(result+len1,str2,len2+1);

  free(str1); <--------- program crashes here with error: invalid pointer
  return result;
}

And the code which calls the function above is like this:

char* str = "John";
str = strjoin(str,"\x20");
str = strjoin(str,"Doe");

In the function strjoin above, I allocate memory for the new string, so I free the old str1. Why is the str1 invalid pointer?

jondinham
  • 8,271
  • 17
  • 80
  • 137
  • 5
    You cannot `free` something you didn't `malloc`. Also, this is basically C code, you should use C++ facilities. – GManNickG Aug 23 '12 at 04:14
  • 1
    `char* str = "John";` your compiler should be warning about a deprecated conversion here. Don't ignore warnings! – Praetorian Aug 23 '12 at 04:21

1 Answers1

5

In the first call of join you are passing a pointer to string literal, which is read-only, to free. You should not be calling free unless you have allocated the memory being freed.

You can fix this in the caller by using strdup:

char* str = strdup("John");
str = strjoin(str,"\x20");
str = strjoin(str,"Doe");
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523