Segfault when using scanf()

Question

I am a beginner with C. This is a simple program that prompts for a name from keyboard input, creates a greeting using the name, then prints it. At runtime, immediately after entering the name at the console and pressing enter, a segmentation fault occurs. After debugging I suspect the fault lies at the scanf() function. I've tried tweaking the argument 'name' with '*' and '&', and initializing the char array 'name' with an empty string, but none of this helped.

// Prompt for a name and print a greeting using the name.    

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

int main()
{
    // Prompt for a name.
    printf("What is your name? ");

    // Get the name.
    char name[20];
    scanf("%s", name);    // Suspect segfault occurs here...

    // Construct the greeting.
    char *greeting;
    char *suffix;
    greeting = "Hello, ";
    suffix = ", nice to meet you!";
    strcat(greeting, name);
    strcat(greeting, suffix);

    // Display the greeting.
    printf("%s", greeting);

    return 0;
}

Answer 1

The two calls to strcat()

char *greeting;
char *suffix;
greeting = "Hello, ";
suffix = ", nice to meet you!";
strcat(greeting, name);
strcat(greeting, suffix);

provoke undefined behaviour by trying to append to storage belonging to a "string"-literal ("Hello, ").

A "string"-literal's storage is

1. constant
2. and even if it weren't, it does not provide any additional room for anything to be appended.

---

To fix this provide a buffer sufficiently large and copy in there everything that is needed:

char *greeting;
char *suffix;
greeting = "Hello, ";
suffix = ", nice to meet you!";
char buffer[7 + 20 + 19 + 1]; /* 7 for "Hello, ", 
                                20 for name (which in fact for your code needed to be 19 only),
                                19 for ", nice to meet you!" and
                                 1 for the 0-terminator. */
strcpy(buffer, greeting); /* Use strcpy() to copy to an uninitialised buffer. */ 
strcat(buffer, name);
strcat(buffer, suffix);

---

Also to make sure the user does not overflow the memory provided for name tell scanf() how much is available;

  char name[20 + 1]; /* If you need 20 characters, define 1 more to hold the
                        "string"'s 0-terminator. */

  scanf("%20s", name); /* Tell scanf() to read in a maximum of 20 chars. */

Answer 2

The problem is with

strcat(greeting, name);

greeting points to read-only memory. Appending name to greeting attempts altering contents in it. The result is segfault.

Answer 3

   strcat(greeting, name);

This call to strcat is modifying a string constant - that's not legal, and that's what's causing your segfault (techincally, what you're seeing is the result of undefined behaviour).

For completeness:

scanf("%s", name);

If the buffer is limited to a size of 20, then you should use:

scanf("%19s", name);

... to limit the number of characters actually stored (though there are better ways to read a variable-length line). I used 19 because there needs to be space for the nul terminator character.

Then, allocate a suitable storage for your complete string:

char full_greeting[20 + 7 + 19] = ""; // name + "hello"... + "nice to meet"...

And copy into that:

strcpy(full_greeting, greeting);
strcat(full_greeting, name);
strcat(full_greeting, suffix);

printf("%s", full_greeting);

Dynamic string solution (POSIX)

On a POSIX system, you can have scanf allocate a buffer for the name it reads:

char *name = NULL;
scanf("%ms", &name); // you could also use 'getline' function
if (name == NULL) {
    exit(1); // scanf failed or memory allocation failed
}

(Note that using getline would read an entire line, which is not the same as the current scanf, which reads a string up to the first whitespace).

Then, you calculate the length of your buffer dynamically:

int req_len = strlen(name) + strlen(greeting) + strlen(suffix) + 1;
// (+1 is for nul terminator)
char * buffer = malloc(req_len);
if (buffer == NULL) {
    exit(1); // or handle the error somehow
}
strcpy(buffer, greeting);
strcat(buffer, name);
strcat(buffer, suffix);

printf("%s", buffer);
free(buffer);
free(name);

Answer 4

char *greeting;
char *suffix;
greeting = "Hello, ";
suffix = ", nice to meet you!";
strcat(greeting, name);
strcat(greeting, suffix);

You are making an strcat when the destination is not only non-modifiable, but also too small. You may know that writing

char *p = "hello";
*p = 'x';

is undefined behavior. This is what strcat is doing to the greeting argument that you pass in. The solution is

#define MAXBUF 64

char *mystrcat(char *dest, char *src)
{
    while (*dest) 
        dest++;
    while (*dest++ = *src++)
        ;

    return --dest;
}

char greeting[MAXBUF], *p;

strcpy(greeting, "hello, ");
p = mystrcat(greeting, name);
mystrcat(p, ", nice to meet you");

Also, notice the new function mystrcat. This is an optimization that is explained in Joel Spolsky's famous post.