fgets doesn't stop reading user input

Question

I found some challenge on reddit to make a program which will sum up all DnD dice rolls. Number of throws is unlimited therefore I created this while loop.

I used fgets to input the string, (I can't input only integers because the input is for example 1d3, where 1 is number of dice thrown, and 3 is number of sides of the dice thrown.)

When the user is prompted to input dice, fgets never stops reading user input.

For example:

To end inputting dice type 0
1d3 
1d4
1d5
0
0
^C

Main function:

int main(void)
{
    char input[MAXSIZE];
    int sum = 0;
    printf("To end inputting dice type 0\n");
    while(*(input) != 0);
    {
        fgets(input, sizeof(input), stdin);
        printf("Debug: input = ");
        puts(input);
        printf("\n");
        sum += dice(input);
        printf("Debug: sum = %d\n", sum);
    }
    printf("Sum of dice rolls is %d.", sum);
    return 0;
}

You have found the culprit (also notice Sourav's correct mention of `0` not being the same as `'0'` in your `while` condition); but there is more that can be improved. You should check that `input[ strlen( input ) -1 ] == '\n'`. If it isn't, the line you read was incomplete. Also: 1) `#include ` missing. 2) `MAXSIZE` undeclared. 3) function `dice` undeclared. Please use [mcve]s. — DevSolar, Jul 16 '18 at 12:49
@DevSolar `input[ strlen( input ) -1 ] == '\n'` sets up a hacker exploit when the first character read is a _null character_. — chux - Reinstate Monica, Jul 16 '18 at 13:28
@chux: Sheesh... good point. I'll let my comment stand though, as the general gist still holds. (Good thing I never wrote C in places where this would have mattered...) — DevSolar, Jul 16 '18 at 13:31

Sourav Ghosh · Answer 1 · 2018-07-16T13:27:58.330

3

Firstly, the literal value of the character input 0 is not 0. In ASCII, it is 48 (decimal).

Try

 while(*(input) != '0')  // (1) - use the character literal form
                         // (2) remove the ;

That said, the standard output is usually line buffered. You need to force a flush if you want to see the outputs in the terminal. You can do that by either

add a newline
```
 printf("Debug: input = \n");
```
use fflush(stdout).

edited Jul 16 '18 at 13:27

answered Jul 16 '18 at 12:22

Sourav Ghosh

133,132
16
183
261

It is a good point that it would make it so that hitting enter without entering a die type would cause it to end if the rest was working, but it's doesn't seem to be the real issue. – Thomas Jager Jul 16 '18 at 12:25
1

@ThomasJager Yes, I added that part, too. – Sourav Ghosh Jul 16 '18 at 12:26
@user3121023 Right you are, missed that. Need to read more carefully. One bug is not enough. :( – Sourav Ghosh Jul 16 '18 at 12:30
2

Detail: C calls `'0'` a _character constant_, not a literal. – chux - Reinstate Monica Jul 16 '18 at 13:26
The code should initialize `input` before testing whether `input[0]` is `'0'`. – Jonathan Leffler Jul 16 '18 at 15:38

score 1 · Answer 2 · answered Jul 16 '18 at 12:25

1

Try this:-

while(fgets(input, sizeof input, stdin) != NULL)

or

while(fgets(input, sizeof input, stdin))

answered Jul 16 '18 at 12:25

Mr. Roshan

1,777
13
33

It doesn't fix the bug. – Lovre Gusar Jul 16 '18 at 12:28
1

@LovreGusar Did you also delete the `;` at the end of the `while()`? – chux - Reinstate Monica Jul 16 '18 at 13:23

score 1 · Answer 3 · answered Jul 16 '18 at 12:34

1

The issue was really simple and such a beginner mistake I feel shameful for even asking the question.

The semicolon after the while loop.

Thanks all for helping me out.

answered Jul 16 '18 at 12:34

Lovre Gusar

51
7

autistic · Answer 4 · 2018-07-18T04:37:32.773

char input[MAXSIZE] = { 0 }; // initialise input!
// otherwise you get to here and access an uninitialised variable:
while(*(input) != 0); // <--- and a semicolon right there!!! Remove that!

In fact I think the loop you want is while (fgets(input, sizeof input, stdin) && strcmp(input, "0\n"))... note that I've hoisted the fgets into the loops control expression.

You should probably do a check after calling fgets to ensure a newline is read, for example

while (fgets(input, sizeof input, stdin) && strcmp(input, "0\n")) {
    size_t n = strcspn(input, "\n");
    if (input[n] == '\n') input[n] = '\0';
    else assert(input[n] == '\0'), // #include <assert.h>
         fscanf(stdin, "%*[^\n]"),
         fgetc(stdin);

There's no undefined behaviour associated with reading unsigned integers when using fscanf, so if you only plan on using positive values you can use that instead of fgets if you wish, i.e.

unsigned dice_count, dice_sides;
while (fscanf(stdin, "%ud%u", &dice_count, &dice_sides) == 2) {
    printf("You chose to roll %u times with dice that contain %u sides\n", dice_count, dice_sides);
}

fgets doesn't stop reading user input

4 Answers4