why does this code crash? [duplicate]

Question

This question already has an answer here:

• Why do I get a segmentation fault when writing to a string initialized with “char *s” but not “char s[]”? 16 answers

why does this code crash? is using strcat illegal on character pointers?

#include <stdio.h>
#include <string.h>

int main()
{
   char *s1 = "Hello, ";
   char *s2 = "world!";
   char *s3 = strcat(s1, s2);
   printf("%s",s3);
   return 0;
}

please give a proper way with referring to both array and pointers.

Answer 1

The problem is that s1 points to a string literal and you are trying to modify it by appending s2 to it. You are not allowed to modify string literals. You need to create a character array and copy both strings into it, like so:

char *s1 = "Hello, ";
char *s2 = "world!";

char s3[100] = ""; /* note that it must be large enough! */
strcat(s3, s1);
strcat(s3, s2);
printf("%s", s3);

"Large enough" means at least strlen(s1) + strlen(s2) + 1. The + 1 is to account for the null terminator.

That having been said, you should seriously consider using strncat (or the arguably better but nonstandard strlcat, if it is available), which are bounds-checked, and thus are far superior to strcat.

Answer 2

The correct way in this case would be to allocate enough space in the destination string(s1) to store 6 extra characters(s2) as well as the null terminator for the string.

char s1[14] = "Hello, ";
char *s2 = "world!";
char *s3 = strcat(s1, s2);
printf("%s",s3);

Answer 3

Here is a quote from the strcat() manual : "The strcat() function appends the src string to the dest string, overwriting the null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result."

The problem here is that s1 and s2 point to static strings which are "read only", so if you try to do a strcat operation, with such a string in the dest parameters, you will get an error.

The best way to create your hello world string here is to malloc it so it will be able to contain both s1 and s2. Also, don't forget to add a '\n' at the end of your printf format string otherwise you might be surprised.

Here is the code I would write if I were you :

int main()
{
  char* s1 = "Hello ";
  char* s2 = "World !";
  char *s3 = malloc((strlen(s1) + strlen(s2) + 1) * sizeof(char));
/* +1 is for the null terminating character
and sizeof(*s3) is the actual size of a char. */

  if (s3)
  {
    strcat(s3, s1);
    strcat(s3, s2);
    printf("%s\n", s3);
    free(s3); // always free what you alloc when you don't need it anymore.
  }
  return 0;
}