Validate the type of input in a do-while loop C
This is a very good example of why scanf should generally not be used for user input.
Since user input is line-based, one would expect that an input function would always read one line of input at a time. However, that is not the way that the function scanf behaves. Instead, it consumes only as many characters as are required to match the %d conversion format specifier. If scanf is unable to match anything, then it does not consume any characters at all, so that the next call to scanf will fail for exactly the same reason (assuming that the same conversion specifier is used and that the invalid input is not explicitly discarded by you). This is what is happening in your code.
At the time of this writing, three of the other answers solve this problem by checking the return value of scanf and explicitly discarding the invalid input. However, all three (!) of these answers have the problem that they for example accept "6sdfj23jlj" as valid input for the number 6, although the whole line of input should obviously be rejected in this case. This is because scanf, as previously mentioned, does not read one line of input at a time.
Therefore, the best solution to your problem would probably be to use line-based input using fgets instead. That way, you will always read exactly one line of input at a time (assuming that the input buffer is large enough to store an entire line of input). After reading the line, you can then attempt to convert it to a number using strtol. Even if the conversion fails, the line of input will have been consumed from the input stream, so you will not have most of the problems described above.
A simple solution using fgets could look like this:
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
int main( void )
{
char line[100];
long number;
//retry until user enters valid input
for (;;) //infinite loop, equivalent to while(1)
{
char *p;
//prompt user for input
printf( "Please enter a number: " );
//attempt to read one line of input
if ( fgets( line, sizeof line, stdin ) == NULL )
{
fprintf( stderr, "Unrecoverable input error!\n" );
exit( EXIT_FAILURE );
}
//attempt to convert input to number
number = strtol( line, &p, 10 );
//verify that conversion was successful
if ( p == line )
{
printf( "Invalid input!\n" );
continue;
}
//verify that remainder of line only contains
//whitespace, so that input such as "6sdfj23jlj"
//gets rejected
for ( ; *p != '\0'; p++ )
{
if ( !isspace( (unsigned char)*p ) )
{
printf( "Encountered invalid character!\n" );
//cannot use `continue` here, because that would go to
//the next iteration of the innermost loop, but we
//want to go to the next iteration of the outer loop
goto continue_outer_loop;
}
}
//input was valid, so break out of infinite loop
break;
//label for breaking out of nested loop
continue_outer_loop:
continue;
}
printf( "Input was valid.\n" );
printf( "The number is: %ld\n", number );
return 0;
}
Note that the goto statement should generally not be used. However, in this case, it is necessary, in order to break out of a nested loop.
This program has the following output:
Please enter a number: 94hjj
Encountered invalid character!
Please enter a number: 5455g
Encountered invalid character!
Please enter a number: hkh7
Invalid input!
Please enter a number: 6sdfj23jlj
Encountered invalid character!
Please enter a number: 67
Input was valid.
The number is: 67
However, this code is still not perfect. It still has the following problems:
If the user enters 100 characters of input in a single line, then the entire line won't fit into the input buffer. In that case, two calls to
fgetswill be required to read the entire line, and the program will incorrectly treat that line as two separate lines of input.The code does not check if the number the user entered is representable as a
long(e.g. whether the number is excessively large). The functionstrtolreports this by settingerrnoaccordingly (which is a feature thatscanflacks).
These two problems can be fixed too, by performing additional checks and error handling. However, the code is now becoming so complex that it makes sense to put it all into its own function:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <limits.h>
#include <errno.h>
int get_int_from_user( const char *prompt )
{
//loop forever until user enters a valid number
for (;;)
{
char buffer[1024], *p;
long l;
//prompt user for input
fputs( prompt, stdout );
//get one line of input from input stream
if ( fgets( buffer, sizeof buffer, stdin ) == NULL )
{
fprintf( stderr, "Unrecoverable input error!\n" );
exit( EXIT_FAILURE );
}
//make sure that entire line was read in (i.e. that
//the buffer was not too small)
if ( strchr( buffer, '\n' ) == NULL && !feof( stdin ) )
{
int c;
printf( "Line input was too long!\n" );
//discard remainder of line
do
{
c = getchar();
if ( c == EOF )
{
fprintf( stderr, "Unrecoverable error reading from input!\n" );
exit( EXIT_FAILURE );
}
} while ( c != '\n' );
continue;
}
//attempt to convert string to number
errno = 0;
l = strtol( buffer, &p, 10 );
if ( p == buffer )
{
printf( "Error converting string to number!\n" );
continue;
}
//make sure that number is representable as an "int"
if ( errno == ERANGE || l < INT_MIN || l > INT_MAX )
{
printf( "Number out of range error!\n" );
continue;
}
//make sure that remainder of line contains only whitespace,
//so that input such as "6sdfj23jlj" gets rejected
for ( ; *p != '\0'; p++ )
{
if ( !isspace( (unsigned char)*p ) )
{
printf( "Unexpected input encountered!\n" );
//cannot use `continue` here, because that would go to
//the next iteration of the innermost loop, but we
//want to go to the next iteration of the outer loop
goto continue_outer_loop;
}
}
return l;
continue_outer_loop:
continue;
}
}
int main( void )
{
int number;
number = get_int_from_user( "Please enter a number: " );
printf( "Input was valid.\n" );
printf( "The number is: %d\n", number );
return 0;
}
Your fundamental mistake is that you never tell your program to consume the invalid input. In words, you told the program:
- Read an integer if there is one. (Otherwise read nothing)
- If you didn't get an integer, go back to step 1.
I assume what you thought you were doing was
- Read the input, and if it was an integer, store it.
- If it wasn't an integer, go back to step 1.
So what you need to do is to rewrite your code so it does what you meant to do (or come up with some other approach, as in the other answer). That is, write your program to:
- Read some amount of input. (e.g. a line)
- Scan the input to see if it is an integer, and store it.
- If it wasn't an integer, go back to step 1.
Some relevant functions you may find useful are fgets, sscanf, and atoi. (also, try to resist the temptation to write buggy code; e.g. if you intend to read a line of input, make sure you actually do so, and correctly. Many people are lazy and will do the wrong thing if the line is long; e.g. just read part of the line, or cause a buffer overflow)
The problem is that "scanf()" can leave unread data in your input buffer. Hence the "infinite loop".
Another issue is that you should validate the return value from scanf(). If you expect one integer value ... and scanf returns "0" items read ... then you know something went wrong.
Here is an example:
#include <stdio.h>
void discard_junk ()
{
int c;
while((c = getchar()) != '\n' && c != EOF)
;
}
int main (int argc, char *argv[])
{
int integer, i;
do {
printf("Enter > ");
i = scanf("%d", &integer);
if (i == 1) {
printf ("Good value: %d\n", integer);
}
else {
printf ("BAD VALUE, i=%i!\n", i);
discard_junk ();
}
} while (i != 1);
return 0;
}
Sample output:
Enter > A
BAD VALUE, i=0!
Enter > B
BAD VALUE, i=0!
Enter > 1
Good value: 1
'Hope that helps!