tags:

views:

827

answers:

6

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 this question and skimmed through other questions, but nothing really answer this specific problem.

+2  A: 

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.)

Teddy
which is nice of it....
rikh
... POSIX anyone? -1.
Tim Post
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.
Tim Post
@tinkertim: the question specifies gcc on Linux, guaranteeing that `strtol` is available
Andomar
Also, at least hinting how to turn such extension ON may help :)
Tim Post
@Andomar: It was getline() that I was taking issue with ;)
Tim Post
+2  A: 

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);
Lucas
"premature optimization is the root of all evil" ... but switch the constants: `'\n'` is much more likely to show up than `EOF` :)
pmg
@pmg: Sure, why not.
Lucas
@pmg: You have over 4000 reps, you should feel free to edit my posts ...
Lucas
You'd hope `EOF` is 100% guaranteed to show up; otherwise, you either have a really fast keyboard or a really slow CPU
Andomar
+2  A: 

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) {
    ...
Andomar
... and deals with a ctrl-d happy user :)
Tim Post
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.
nuuz
Your `while` loop will consume everything, `'\n'` included.
pmg
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.
nuuz
+7  A: 

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.

Jergason
If the input is "abc", that code will print "Err. . ." three times.
Teddy
Yeah, it is pretty ghetto. I will tweak it a bit.
Jergason
Now if the input is "ab-10", it will incorrectly remove the minus sign from the input and read "10" as the next number.
caf
Just goes to show that `scanf` is the spawn of the devil ;-)
RBerteig
A: 

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).

Eli Bendersky
+2  A: 

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);
    }
/* ... */
pmg
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).
Roman Nikitchenko