I am getting a SegFault error, but why?

Question

I am getting a Segmentation Fault error when I run the code, but other than that it compiles and runs. If you know why the error is occurring, I'd appreciate the help. Also please explain why it is occurring, as I'm curious.

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

int main()
{
    float att,def,hp,agi,stl,wis,ran,acc;
    char name[10],contents[100];
    int warrior,lvl=1,kills=0;

    printf("What is your name?\n");
    gets(name);

    printf("1: Ninja\n");
    printf("2: Knight\n");
    printf("3: Archer\n\n");
    printf("Pick a warrior.\n");
    scanf("%i",warrior);

    ...

    char attack[10],defense[10],health[10],agility[10],stealth[10],wisdom[10],range[10],accuracy[10],level[10],kill[10];
    snprintf(attack,10,"%f",att);
    snprintf(defense,10,"%f",def);
    snprintf(health,10,"%f",hp);
    snprintf(agility,10,"%f",agi);
    snprintf(stealth,10,"%f",stl);
    snprintf(wisdom,10,"%f",wis);
    snprintf(range,10,"%f",ran);
    snprintf(accuracy,10,"%f",acc);
    snprintf(level,10,"%f",lvl);
    snprintf(kill,10,"%f",kills);   

    char my_path[25];
    strcat(my_path,"Warriors/");
    strcat(my_path,name);
    strcat(my_path,".txt");


    FILE *fp;
    fp = fopen(my_path, "w+");

    fputs(attack, fp);
    fputs(" ", fp);
    fputs(defense, fp);
    fputs(" ", fp);
    fputs(health, fp);
    fputs(" ", fp);
    fputs(agility, fp);
    fputs(" ", fp);
    fputs(stealth, fp);
    fputs(" ", fp);
    fputs(wisdom, fp);
    fputs(" ", fp);
    fputs(range, fp);
    fputs(" ", fp);
    fputs(accuracy, fp);
    fputs(" ", fp);
    fputs(level, fp);
    fputs(" ", fp);
    fputs(kill, fp);

    fclose(fp);
}

Welcome to Stack Overflow. What you've got is a trivial error (nonetheless common and with serious consequences). One of the things you need to do is learn how to create an MCVE ([MCVE]) — doing so would have given you a program of under 10 lines to show, and you might even have worked out what was wrong on your own. Please note that [`gets()` is too dangerous to be used — ever](http://stackoverflow.com/questions/1694036/). And be cautious about mixing line inputs like `fgets()` with `scanf()` because [`scanf()` leaves the newline in the buffer](https://stackoverflow.com/questions/5240789/) — Jonathan Leffler, Aug 12 '16 at 02:47
when calling any of the `scanf` family of functions, always check the returned value (not the parameter value) to assure the operation was successful. — user3629249, Aug 13 '16 at 03:39
when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful. — user3629249, Aug 13 '16 at 03:40

sps · Accepted Answer · 2016-08-12T03:47:40.127

There are following issues in the code.

Incorrect use of scanf while reading value for warrior.

/* scanf("%i",warrior); */ // Put '&' before warrior
scanf("%i",&warrior);

Incorrect format specifiers in snprintf while printing values of integers lvl, and kills.

/* snprintf(level,10,"%f",lvl); */ // Change %f to %d
snprintf(level,10,"%d",lvl);
/* snprintf(kill,10,"%f",kills); */ // Change %f to %d
snprintf(kill,10,"%d",kills);

Using strcat with my_path without initializing it.

/* char my_path[25]; */ // Initialize as seen below 
char my_path[25] = ""; 
strcat(my_path,"Warriors_");
strcat(my_path,name);
strcat(my_path,".txt");

Finally, you are using gets, which should be avoided. You can change it to fgets.

Now coming to why your program is crashing.

First when you did scanf("%i",warrior);: Instead of the address (i.e. &warrior) you gave the value of warrior variable. And since warrior was not initialized, it had an indeterminate value. Now scanf treated the value of warrior as an address and tried to write the value there. And, since the value of warrior was indeterminate, it invoked undefined behavior.

Second when you used %f instead of %d (mismatch of format specifier) in snprintf, the code again invoked undefined behavior.

Third, when you use strcat without initializing my_path, your array my_path will have indeterminate values. Now, strcat looks for value 0 starting from the address pointed by my_path and keeps on looking untill it finds a 0. While doing this it may try to read value outside the array, again invoking undefined behavior.

When undefined behavior is invoked, anything can happen. In your case you got a segmentation fault.

P.S.: You should also check return value of scanf to check if it succeded or not.

P.S.: You should also check return value of fopen to check if it succeded or not.

score 1 · Answer 2 · answered Aug 12 '16 at 02:33

1

scanf("%i",warrior);

scanf expects a pointer. http://linux.die.net/man/3/scanf

Fix it to

scanf("%i", &warrior);

answered Aug 12 '16 at 02:33

lost_in_the_source

10,998
9
46
75

3

One more: `char my_path[25];` --> `char my_path[25] = "";`. Because OP uses `strcat` on that buffer. – kaylum Aug 12 '16 at 02:34

score 0 · Answer 3 · answered Aug 12 '16 at 03:23

0

you'd better also add this kind of code to check if fopen is successful.

  if(fp==NULL)
  {   
      printf("can't open file:%s\n",my_path);
  }   
  else
  {
      ......
  }

answered Aug 12 '16 at 03:23

peter__barnes

351
2
5

score 0 · Answer 4 · answered Aug 13 '16 at 03:36

these two lines:

char my_path[25];
strcat(my_path,"Warriors/");

are incorrect because the strcat() function appends to the end of the current string (where it finds a NUL char '\0')

There is no telling where that NUL char will be found.

Suggest using:

char my_path[25];
strcpy(my_path,"Warriors/");

I am getting a SegFault error, but why?

4 Answers4