Why is scanf() causing infinite loop in this code?
Asked Answered
H

16

55

I've a small C-program which just reads numbers from stdin, one at each loop cycle. If the user inputs some NaN, an error should be printed to the console and the input prompt should return again. On input of "0", the loop should end and the number of given positive/negative values should be printed to the console. Here's the program:

#include <stdio.h>

int main()
{
    int number, p = 0, n = 0;

    while (1) {
        printf("-> ");
        if (scanf("%d", &number) == 0) {
            printf("Err...\n");
            continue;
        }
        
        if (number > 0) p++;
        else if (number < 0) n++;
        else break; /* 0 given */
    }

    printf("Read %d positive and %d negative numbers\n", p, n);
    return 0;
}

My problem is, that on entering some non-number (like "a"), this results in an infinite loop writing "-> Err..." over and over. I guess it's a scanf() issue and I know this function could be replace by a safer one, but this example is for beginners, knowing just about printf/scanf, if-else and loops.

I've already read the answers to the questionscanf() skips every other while loop in C and skimmed through other questions, but nothing really answer this specific problem.

Hectometer answered 11/11, 2009 at 15:39 Comment(4)
Many closely related SO questions, including: stackoverflow.com/questions/1669821Buddhology
In response to all the answers and hints: Adding while (getchar() != '\n'); before "continue" inside the if-statement works really fine for me and (hopefully) solves all/most of the problems. Further, it's reasonable explainable to beginners :).Hectometer
See also Using fflush(stdin).Buddhology
Note that the loop mentioned by user208785 in the comment above should be int c; while ((c = getchar()) != EOF && c != '\n') ; — The type of c should be int and the code needs to test both EOF and newline, though the code will normally get a newline before EOF.Buddhology
D
42

scanf consumes only the input that matches the format string, returning the number of characters consumed. Any character that doesn't match the format string causes it to stop scanning and leaves the invalid character still in the buffer. As others said, you still need to flush the invalid character out of the buffer before you proceed. This is a pretty dirty fix, but it will remove the offending characters from the output.

char c = '0';
if (scanf("%d", &number) == 0) {
  printf("Err. . .\n");
  do {
    c = getchar();
  }
  while (!isdigit(c));
  ungetc(c, stdin);
  //consume non-numeric chars from buffer
}

edit: fixed the code to remove all non-numeric chars in one go. Won't print out multiple "Errs" for each non-numeric char anymore.

Here is a pretty good overview of scanf.

Disorganization answered 11/11, 2009 at 15:47 Comment(9)
If the input is "abc", that code will print "Err. . ." three times.Butterscotch
Yeah, it is pretty ghetto. I will tweak it a bit.Disorganization
Now if the input is "ab-10", it will incorrectly remove the minus sign from the input and read "10" as the next number.Literate
I know it's old, but just change it to while (!isdigit(c) && c != '-');, that should also help with minus signs.Vinificator
This still results in multiple input lines, try 4t and t4, 4t will give you -> Err. . . and t4 will not even give you any errors, but still multiple input lines: -> -> Acyclic
You better put the getchar() just before the printf("Err. . .\n");Acyclic
returning the number of characters consumed -- Actually, scanf returns the number of fields successfully read. So scanf("%d") might return 1, 0, or EOF.Please
Despite the number of up-votes, this is not a great answer. The condition should be if (scanf("%d", &number) != 1) because you could get EOF or 0. You might need to capture the return value (int rv; if ((rv = scanf(…)) != 1)) to distinguish between EOF and error (another option would be feof(), which might actually be correct here but usually isn't). The variable c should be an int, not a char. The loop should probably be: int c; while ((c = getchar()) != EOF && c != '\n' && !isdigit(c)) ; (where I'd place the semicolon on a line on its own to indicate an empty loop body).Buddhology
Note that if there's an erroneous character in the input, it's likely that the rest of the line is not useful. It is usual to read to the end of the line, rather than to stop on a digit after the faulty character. If someone type a6, it is not clear that they meant to type 6 and added the a — it is probably better to remove all the data. One advantage of the technique of reading a whole line with fgets() or POSIX getline() and then scanning it with sscanf() is that you can report errors better. See also Using sscanf() in a loop.Buddhology
B
10

scanf() leaves the "a" still in the input buffer for next time. You should probably use getline() to read a line no matter what and then parse it with strtol() or similar instead.

(Yes, getline() is GNU-specific, not POSIX. So what? The question is tagged "gcc" and "linux". getline() is also the only sensible option to read a line of text unless you want to do it all by hand.)

Butterscotch answered 11/11, 2009 at 15:41 Comment(7)
You can not rely on non standard extensions for something as crucial as user input without providing them in your own tree in case they do not exist. If you edit your answer to reflect this, I will withdraw my down vote.Scarlettscarp
@tinkertim: the question specifies gcc on Linux, guaranteeing that strtol is availableSkirret
Also, at least hinting how to turn such extension ON may help :)Scarlettscarp
@Andomar: It was getline() that I was taking issue with ;)Scarlettscarp
@TimPost Both getline() and getdelim() were originally GNU extensions. They were standardized in POSIX.1-2008.Detonation
suggesting to use getline() will break portability of the code.Acyclic
If you use POSIX getline() to read a line, you must remember to free() the space (or, at least, consider whether it needs freeing, and the answer will usually be "yes" so you end up freeing it). Note that getline() typically allocates space even if the first operation returns -1 (indicating EOF; note that it explicitly returns -1 and not EOF, even though EOF is usually -1).Buddhology
A
9

I think you just have to flush the buffer before you continue with the loop. Something like that would probably do the job, although I can't test what I am writing from here:

int c;
while((c = getchar()) != '\n' && c != EOF);
Accommodative answered 11/11, 2009 at 15:42 Comment(4)
"premature optimization is the root of all evil" ... but switch the constants: '\n' is much more likely to show up than EOF :)Barye
You'd hope EOF is 100% guaranteed to show up; otherwise, you either have a really fast keyboard or a really slow CPUSkirret
The above complication in the while statement's condition is unnecessary.Acyclic
@Acyclic What do you mean? It looks fine to me.Jollanta
S
4

Due to the problems with scanf pointed out by the other answers, you should really consider using another approach. I've always found scanf way too limited for any serious input reading and processing. It's a better idea to just read whole lines in with fgets and then working on them with functions like strtok and strtol (which BTW will correctly parse integers and tell you exactly where the invalid characters begin).

Sack answered 11/11, 2009 at 15:53 Comment(0)
B
4

Rather than using scanf() and have to deal with the buffer having invalid character, use fgets() and sscanf().

/* ... */
    printf("0 to quit -> ");
    fflush(stdout);
    while (fgets(buf, sizeof buf, stdin)) {
      if (sscanf(buf, "%d", &number) != 1) {
        fprintf(stderr, "Err...\n");
      } else {
        work(number);
      }
      printf("0 to quit -> ");
      fflush(stdout);
    }
/* ... */
Barye answered 11/11, 2009 at 16:0 Comment(1)
fgets() read some buffer and if it doesn't contain format right from the beginning the whole line is thrown away. This could be not acceptable (but could be desired, it depends on requirements).Dorsman
K
3

I had similar problem. I solved by only using scanf.

Input "abc123<Enter>" to see how it works.

#include <stdio.h>
int n, num_ok;
char c;
main() {
    while (1) {
        printf("Input Number: ");
        num_ok = scanf("%d", &n);
        if (num_ok != 1) {
            scanf("%c", &c);
            printf("That wasn't a number: %c\n", c);
        } else {
            printf("The number is: %d\n", n);
        }
    }
}
Kalmick answered 14/9, 2012 at 13:25 Comment(1)
This still doesn't solve the problem entirely, since if you enter a combination of alphanumeric characters, like 6y: Input Number: 6y will result in : The number is: 6 Input Number: That wasn't a number: y the program reads the input character by character, when it finds a number character in the input, it thinks the input is a number, and when it finds a non numeric one it thinks it is not a number, but can not decide that 6y is not a number altogether, and of course in the process due to the [Enter] key being still present in the buffer, the same problem arises.Acyclic
B
1

On some platforms (especially Windows and Linux) you can use fflush(stdin);:

#include <stdio.h>

int main(void)
{
  int number, p = 0, n = 0;

  while (1) {
    printf("-> ");
    if (scanf("%d", &number) == 0) {
        fflush(stdin);
        printf("Err...\n");
        continue;
    }
    fflush(stdin);
    if (number > 0) p++;
    else if (number < 0) n++;
    else break; /* 0 given */
  }

  printf("Read %d positive and %d negative numbers\n", p, n);
  return 0;
}
Bobodioulasso answered 25/10, 2012 at 16:25 Comment(5)
Please read Using fflush(stdin) — especially the comments to the question — for information about this. It works on Windows because Microsoft documents that it does; it doesn't work anywhere else (that I know of) in practice, notwithstanding some documentation suggesting the contrary.Buddhology
It works on Linux now (or I should say glibc). It didn't before, and I don't know when they changed it. But the last time I tried it on a mac it crashed, and it is not in the standard, so I've added a warning about portability to this answer.Nervy
Not working for me here with my current version. $ ldd --version gives ldd (Debian GLIBC 2.19-18+deb8u9) 2.19. That should give all info needed. Anyone has a clue why?Breach
fflush input stream is only defined for input streams associated with seekable files (e.g., disk files, but not pipes or terminals). POSIX.1-2001 did not specify the behavior for flushing of input streams, POSIX.1-2008 does, but only in the limited way described.Plumbum
using fflush(stdin) will cause undefined behavior and is not guaranteed to work portably.Acyclic
P
1

The Solution: You need to add fflush(stdin); when 0 is returned from scanf.

The Reason: It appears to be leaving the input char in the buffer when an error is encountered, so every time scanf is called it just keeps trying to handle the invalid character but never removing it form the buffer. When you call fflush, the input buffer(stdin) will be cleared so the invalid character will no longer be handled repeatably.

You Program Modified: Below is your program modified with the needed change.

#include <stdio.h>

int main()
{
    int number, p = 0, n = 0;

    while (1) {
        printf("-> ");
        if (scanf("%d", &number) == 0) {
            fflush(stdin);
            printf("Err...\n");
            continue;
        }

        if (number > 0) p++;
        else if (number < 0) n++;
        else break; /* 0 given */
    }

    printf("Read %d positive and %d negative numbers\n", p, n);
    return 0;
}
Phoenicia answered 26/7, 2019 at 17:10 Comment(1)
See Using fflush(stdin) for a disquisition on why using fflush(stdin) anywhere other than Windows is fraught.Buddhology
A
0

I had the same problem, and I found a somewhat hacky solution. I use fgets() to read the input and then feed that to sscanf(). This is not a bad fix for the infinite loop problem, and with a simple for loop I tell C to search for any none numeric character. The code below won't allow inputs like 123abc.

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

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

    char line[10];
    int loop, arrayLength, number, nan;
    arrayLength = sizeof(line) / sizeof(char);
    do {
        nan = 0;
        printf("Please enter a number:\n");
        fgets(line, arrayLength, stdin);
        for(loop = 0; loop < arrayLength; loop++) { // search for any none numeric charcter inisde the line array
            if(line[loop] == '\n') { // stop the search if there is a carrage return
                break;
            }
            if((line[0] == '-' || line[0] == '+') && loop == 0) { // Exculude the sign charcters infront of numbers so the program can accept both negative and positive numbers
                continue;
            }
            if(!isdigit(line[loop])) { // if there is a none numeric character then add one to nan and break the loop
                nan++;
                break;
            }
        }
    } while(nan || strlen(line) == 1); // check if there is any NaN or the user has just hit enter
    sscanf(line, "%d", &number);
    printf("You enterd number %d\n", number);
    return 0;
}
Acyclic answered 30/12, 2013 at 8:38 Comment(0)
B
0

try using this:

if (scanf("%d", &number) == 0) {
        printf("Err...\n");
        break;
    }

this worked fine for me... try this.. the continue statement is not appropiate as the Err.. should only execute once. so, try break which I tested... this worked fine for you.. i tested....

Boatyard answered 7/10, 2016 at 5:34 Comment(0)
A
0

When a non-number is entered an error occurs and the non-number is still kept in the input buffer. You should skip it. Also even this combination of symbols as for example 1a will be read at first as number 1 I think you should also skip such input.

The program can look the following way.

#include <stdio.h>
#include <ctype.h>

int main(void) 
{
    int p = 0, n = 0;

    while (1)
    {
        char c;
        int number;
        int success;

        printf("-> ");

        success = scanf("%d%c", &number, &c);

        if ( success != EOF )
        {
            success = success == 2 && isspace( ( unsigned char )c );
        }

        if ( ( success == EOF ) || ( success && number == 0 ) ) break;

        if ( !success )
        {
            scanf("%*[^ \t\n]");
            clearerr(stdin);
        }
        else if ( number > 0 )
        {
            ++p;
        }
        else if ( number < n )
        {
            ++n;
        }
    }

    printf( "\nRead %d positive and %d negative numbers\n", p, n );

    return 0;
}

The program output might look like

-> 1
-> -1
-> 2
-> -2
-> 0a
-> -0a
-> a0
-> -a0
-> 3
-> -3
-> 0

Read 3 positive and 3 negative numbers
Argeliaargent answered 9/2, 2017 at 19:55 Comment(0)
N
0

To solve partilly your problem I just add this line after the scanf:

fgetc(stdin); /* to delete '\n' character */

Below, your code with the line:

#include <stdio.h>

int main()
{
    int number, p = 0, n = 0;

    while (1) {
        printf("-> ");
        if (scanf("%d", &number) == 0) {
            fgetc(stdin); /* to delete '\n' character */
            printf("Err...\n");
            continue;
        }

        if (number > 0) p++;
        else if (number < 0) n++;
        else break; /* 0 given */
    }

    printf("Read %d positive and %d negative numbers\n", p, n);
    return 0;
}

But if you enter more than one character, the program continues one by one character until the "\n".

So I found a solution here: How to limit input length with scanf

You can use this line:

int c;
while ((c = fgetc(stdin)) != '\n' && c != EOF);
Nardoo answered 12/5, 2019 at 1:19 Comment(0)
B
0
// all you need is to clear the buffer!

#include <stdio.h>

int main()
{
    int number, p = 0, n = 0;
    char clearBuf[256]; //JG:
    while (1) {
        printf("-> ");
        if (scanf("%d", &number) == 0) {
            fgets(stdin, 256, clearBuf); //JG:
            printf("Err...\n");
            continue;
        }

        if (number > 0) p++;
        else if (number < 0) n++;
        else break; /* 0 given */
    }

    printf("Read %d positive and %d negative numbers\n", p, n);
    return 0;
}
Bouley answered 3/7, 2020 at 13:48 Comment(0)
S
-1

Flush the input buffer before you scan:

while(getchar() != EOF) continue;
if (scanf("%d", &number) == 0) {
    ...

I was going to suggest fflush(stdin), but apparently that results in undefined behavior.

In response to your comment, if you'd like the prompt to show up, you have to flush the output buffer. By default, that only happens when you print a newline. Like:

while (1) {
    printf("-> ");
    fflush(stdout);
    while(getchar() != EOF) continue;
    if (scanf("%d", &number) == 0) {
    ...
Skirret answered 11/11, 2009 at 15:43 Comment(3)
Adding this while-loop before the if-statement does result in a wrong program behaviour. To be exact, the "->" prompt isn't shown after the first input, may it either be right or wrong.Hectometer
Your while loop will consume everything, '\n' included.Barye
Afaik fflush() doesn't work the same way on each system. At least on my Linux box, the fflush(stdout) doesn't help to show up the "->" prompt. Also, a call to setvbuf() doesn't help here, too.Hectometer
P
-1

Hi I know this is an old thread but I just finished a school assignment where I ran into this same problem. My solution is that I used gets() to pick up what scanf() left behind.

Here is OP code slightly re-written; probably no use to him but perhaps it will help someone else out there.

#include <stdio.h>

    int main()
    {
        int number, p = 0, n = 0;
        char unwantedCharacters[40];  //created array to catch unwanted input
        unwantedCharacters[0] = 0;    //initialzed first byte of array to zero

        while (1)
        {
            printf("-> ");
            scanf("%d", &number);
            gets(unwantedCharacters);        //collect what scanf() wouldn't from the input stream
            if (unwantedCharacters[0] == 0)  //if unwantedCharacters array is empty (the user's input is valid)
            {
                if (number > 0) p++;
                else if (number < 0) n++;
                else break; /* 0 given */
            }
            else
                printf("Err...\n");
        }
        printf("Read %d positive and %d negative numbers\n", p, n);
        return 0;
    }
Pulp answered 16/3, 2015 at 13:1 Comment(2)
gets is horribly unsafe and should never be used (it has been removed from standard C for that reason).Guck
i agree it is dangerous friend and i only used it here for a small application (hence the subjective 40 char array). if the problem at hand were more objective in its requirements then ya ^^.Pulp
C
-1

I've recently been through the same problem, and I found a solution that might help a lot of people. The function "scanf" leaves a buffer in memory ... and that's why the infinite loop is caused. So you actually have to "store" this buffer to another variable IF your initial scanf contains the "null" value. Here's what I mean:

#include <stdio.h>
int n;
char c[5];
int main() {
    while (1) {
        printf("Input Number: ");
        if (scanf("%d", &n)==0) {  //if you type char scanf gets null value
            scanf("%s", &c);      //the abovementioned char stored in 'c'
            printf("That wasn't a number: %s\n", c);
        }
        else printf("The number is: %d\n", n);
    }
}
Craigcraighead answered 14/12, 2016 at 21:56 Comment(1)
scanf("%s", &c) is a type error. %s takes a char *, not a char (*)[5]. Also, since you're not limiting the number of characters read, this is a buffer overflow waiting to happen. Simply discarding the input would be a much better idea (%*s).Guck

© 2022 - 2024 — McMap. All rights reserved.