why do i get Segmentation fault executing this code?

Question

I'm trying to figure out the issue with this code:

int main(int argc, char *argv[])
{
    char *s;
    scanf("%s\n", s);

    char *t = malloc(sizeof(*s) + 1);

    strcpy(t, s);
    t[0] = toupper(t[0]);
    printf("s: %s\n", s);
    printf("t: %s\n", t);

    free(t);
    return 0;
}

I was trying to upper case the first alphabet of my input to get for example hi! changed into Hi!.

Answer 1

1. You are using scanf on a char* without first allocating memory to it. This may lead to segmentation fault. In fact, the following code :

   #include<stdio.h>
   int main()
   {
       char* str;
       scanf("%s", str);
       printf("%s\n", str);
       return 0;
   }
   

   does give segmentation fault sometimes. Instead you can use a char array instead of a char* if you want to avoid malloc before scanf . You can use fgets instead of scanf to prevent writing out of bounds or you can check with scanf itself like this:

   char s[100];
   if(scanf("%99s",s) != 1) { /* error */ }
   

   Also in your code, you have a \n after %s in the scanf statement, that I have removed(assuming you didn't intend this behaviour) . You can see what that does here: Using "\n" in scanf() in C

2. Secondly, in allocating memory for t, you are using sizeof(*s) which is just sizeof(char) which is defined to be 1. Instead, you should be using strlen(s)+1 which will allocate one more byte than the length of s which should work fine. Also, checking if t==NULL is also recommended.

3. Before calling toupper the argument must be type-casted into an unsigned char to avoid the risks of undefined behaviour if the char type is signed on the platform. More on this can be found on this answer: https://stackoverflow.com/a/21805970/18639286 (Though the question was a C++ question, the answer actually quotes C standard specifications)

I have edited the above changes into your code to get the following:

int main (int argc,char* argv[])
{

    char s[100];
    if(scanf("%99s",s)!=1)
    {
       fprintf(stderr, "Invalid or missing input\n");
       // scanf should have returned 1 if it successfully read 1 input
    }
    char *t = malloc(strlen(s) + 1); // to store the letters + '\0' at the end
    if(t==NULL)
    {
       fprintf(stderr, "Memory allocation failed\n");
       exit(1);
    }

    strcpy(t, s);
    t[0] = toupper((unsigned char)t[0]);
    printf("s: %s\n", s);
    printf("t: %s\n", t);

    free(t);
    return 0;

}

Some extra points:

You can also use fgets instead of scanf : fgets(s, 100, stdin); as fgets does bound checking.

Also, fgets reads till end of line/file or till the buffer is full, so it will read past the spaces in input, But scanf will stop at a space. Or as Ted Lyngmo pointed out in the comments, scanf with %s reads a word, while fgets reads a line.

But, even if you intend to read only a word, you can use fgets and then use sscanf to read the first word from the string with which you used fgets. Example:

char buffer[20];
char str[sizeof buffer]=""; //initialize with an empty string
if(fgets(buffer, sizeof buffer, stdin)==NULL)
{
  fprintf(stderr, "Error reading input\n");
  exit(1);
}
if(sscanf(buffer, "%s", str)!=1)
{
  // if str was not read
  fprintf(stderr, "String not read\n");
  exit(1);
}
// DO stuff with str 

Here, you don't need to use %19s because we know the size of buffer.

Answer 2

As a sidenote to the already great answer, I must recommend GNU Debugger (shortly gdb).

Run "gdb [program name]", the program should be compiled with the -g flag. In the interactive environment, you can run that program by typing r which may yield some useful information. Typing "b <number>" to specify a break in the runtime, then type "s [number]" a number of lines ahead in the program.

There are plenty of ways with gdb to extract information about the running program, I'd heavily recommend reading the docs on it.