-2

I'm trying to use fgets to get a line from stdin. Here's my code

char* FENString;
printf("Enter FEN Key: ");
fgets(FENString, 50, stdin);

FENString only has one char, and that's the new line character. I've tried looking for help and haven't found anything, does anyone know why this is happening?


It's very important to make sure that all variables have some sort of memory allocated to it, at least in some point of its lifecycle. The issue here was that the char pointer didn't have any sort of memory allocated to it. Something that could've fixed it is mallocing the FENString, or perhaps changing the declaration to something like char FENString[50];

Matthew Kerian
  • 812
  • 5
  • 18
  • Possible duplicate of [Why does a program accessing illegal pointer to pointer not crash?](https://stackoverflow.com/questions/17852212/why-does-a-program-accessing-illegal-pointer-to-pointer-not-crash) Not an exact duplicate, but the explanation for the underlying behavior is there. – Mogsdad Dec 10 '17 at 15:14

2 Answers2

3

You are having Undefined behavior by providing a unintialized pointer to fgets. Allocate memory of >=50 size to FENString and then pass it to fgets.

Solution-1

char* FENString;
FENString = malloc(50);
if( FENString == NULL){
    fprintf(stderr,"%s\n","Error in malloc");
    exit(1);
}
printf("Enter FEN Key: ");
if( fgets(FENString, 50, stdin) == NULL){
    fprintf(stderr,"%s\n","Error in input");
    exit(1);
}
// Work with FENString;
...
free(FENString);

Solution-2

Simply have a char array like this char FENString[50]; Then the code would be

char FENString[50];
printf("Enter FEN Key: ");
if( fgets(FENString, sizeof FENString, stdin) == NULL){
    fprintf(stderr,"%s\n","Error in input");
    exit(1);
}
// Work with FENString

Instead of 50 here sizeof FENString although it does the same thing this is much better towards a good maintainable solution. In case you letter change it, you don't have to search and replace 50 it will be automaticlaly done due to use of sizeof.(Peter points out)

Community
  • 1
  • 1
user2736738
  • 30,591
  • 5
  • 42
  • 56
  • 1
    @user3386109.: Thanks for the edit. Appreciate your time and help. – user2736738 Dec 09 '17 at 07:51
  • In the "solution-2", it is possible to use `sizeof(FENString)` rather than `50` in the `fgets()` call. This ensures the correct value is passed regardless of what dimension `FENString` is defined with (so, if the dimension of the array is changed, it is not necessary to remember to change the length specified to `fgets()`). – Peter Dec 09 '17 at 07:57
  • @Peter.: Yes that's a nice pick. I edited. Thanks for the suggestion. – user2736738 Dec 09 '17 at 07:58
  • @Peter.: Added explanation along with it. Thanks again – user2736738 Dec 09 '17 at 08:01
0

Here

char* FENString;
printf("Enter FEN Key: ");
fgets(FENString, 50, stdin);

FENString is a character pointer and before putting any data into it, FENString should have valid memory space to store something into it. But in your case FENString doesn't point to valid address which is the issue.

To resolve this you can take FENString either as a character buffer like

char FENString[1024] = {}; 

or allocate memory dynamically for FENString like

char* FENString = malloc(sizeof(*FENString) * MAX_BUF_SIZE); /* define the MAX_BUF_SIZE, instead of magic number like 50 use macro */
/* Do error handling malloc() */
if(FENString == NULL){
    fprintf(stderr, "malloc() failed\n");
    exit(1);
}

Now store the user input into dynamically created array by calling fgets() . For example

if(fgets(FENString, MAX_BUF_SIZE, stdin) != NULL){
    /* fgets is success, but if fgets store the \n at the end of buffer if its read */
    /* to remove trailing \n char use strcspn() */
    FENString[strcspn(FENString, "\n")] = 0;
}
Achal
  • 11,821
  • 2
  • 15
  • 37