Search code examples
cvalidationscanfinfinite-loop

C infinite loop when char input instead of int


I have a C program which is supposed to validate that the input from the user is an int between 1 and 8. It works if an integer is entered, but when characters are entered the validation loop repeats forever. Can you tell what I am doing wrong?

#include <stdio.h>

int main(void)
{
  int i;
  int input;
  domainEntry *myDomains = buildDomainDB();

  printf("You have the choice between the"
         " following top domains: 1-EDU, 2-COM"
         ", 3-ORG, 4-GOV, 5-MIL, 6-CN, 7-COM.CN, 8.CAN\n");
  printf("Which one do you want to pick?");
  scanf(" %d", &input);

  if(input < 1 || input > 9)
  {
    do  
    {   
      printf("Invalid input. Please try again.");
      printf("You have the choice between the"
             " following top domains: 1-EDU, 2-COM"
             ", 3-ORG, 4-GOV, 5-MIL, 6-CN, 7-COM.CN, 8.CAN\n");
      printf("Which one do you want to pick?");
      scanf(" %d", &input);

    }while((input < 1) || (input > 8));
  }
}

Solution

  • Any time you are taking user-input, you must account for each character that remains in the input buffer (stdin here). This is especially true when taking input with scanf (or family) due to the way scanf handles input or matching failures. When either occurs, no further characters are read, and any offending characters are left in the input buffer unread -- just waiting to bite you again on your next attempted read (generally resulting in an infinite loop if you are taking input within a loop)

    Further, you must understand how each conversion specier handles leading whitespace and whether leading whitespace is consumed by the specifier (e.g. numeric conversion specifiers and "%s") and which do not (all others, particularly "%c" and "%[...]").

    This is one of the primary reasons a line-oriented function such as fgets (with an adequately sized buffer) or POSIX getline are recommended for taking user input. Both read and include the trailing '\n' in the buffer filled. This consumes the line completely, eliminating any uncertainty which additional characters remain in the input buffer unread. You can then pass the buffer to sscanf for parsing. This allows independent validation of both (1) the read ("did I get input?") and (2) the parse of information from the line ("did it contain the information I need?").

    scanf can be used, if used correctly. This means you are responsible for checking the return of scanf every time. You must handle three conditions

    1. (return == EOF) the user canceled input by generating a manual EOF by pressing Ctrl+d (or on windows Ctrl+z, but see CTRL+Z does not generate EOF in Windows 10 (early versions));
    2. (return < expected No. of conversions) a matching or input failure occurred. For a matching failure you must account for every character that will be left in your input buffer. (scan forward in the input buffer reading and discarding characters until a '\n' or EOF is found); and finally
    3. (return == expected No. of conversions) indicating a successful read -- it is then up to you to check whether the input meets any additional criteria (e.g. positive integer, positive floating-point, within a needed range, etc..).

    Utilizing that with your code, you could take input in the following way -- looping continually until valid input is received or the user cancels by generating a manual EOF, e.g.

    #include <stdio.h>
    
    void empty_stdin (void) /* simple helper-function to empty stdin */
    {
        int c = getchar();
    
        while (c != '\n' && c != EOF)
            c = getchar();
    }
    
    int main(void)
    {
        int input = 0,
            rtn = 0;    /* variable to save scanf return */
        // domainEntry *myDomains = buildDomainDB();
    
        for (;;) {  /* loop continually until valid input or EOF */
            printf ("\nSelect top level domain:\n"
                    "  1-EDU\n"
                    "  2-COM\n"
                    "  3-ORG\n"
                    "  4-GOV\n"
                    "  5-MIL\n"
                    "  6-CN\n"
                    "  7-COM.CN\n"
                    "  8.CAN\n\n"
                    "choice: ");
            rtn = scanf (" %d", &input);    /* save return */
    
            if (rtn == EOF) {   /* user generates manual EOF */
                fputs ("(user canceled input.)\n", stderr);
                return 1;
            }
            else if (rtn == 0) {    /* matching failure */
                fputs (" error: invalid integer input.\n", stderr);
                empty_stdin();
            }
            else if (input < 1 || 8 < input) {  /* validate range */
                fputs (" error: integer out of range [1-8]\n", stderr);
                empty_stdin();
            }
            else {  /* good input */
                empty_stdin();
                break;
            }
        }
    
        printf ("\nvalid input: %d\n", input); 
    }
    

    (note the use of the helper-function empty_stdin() -- and that even after a successful read, you should still empty_stdin() to insure the input buffer is empty and prepared for the next user input -- whatever that may be)

    Example Use/Output

    $ ./bin/getintmenu
    
    Select top level domain:
      1-EDU
      2-COM
      3-ORG
      4-GOV
      5-MIL
      6-CN
      7-COM.CN
      8.CAN
    
    choice: edu
     error: invalid integer input.
    
    Select top level domain:
      1-EDU
      2-COM
      3-ORG
      4-GOV
      5-MIL
      6-CN
      7-COM.CN
      8.CAN
    
    choice: 9
     error: integer out of range [1-8]
    
    Select top level domain:
      1-EDU
      2-COM
      3-ORG
      4-GOV
      5-MIL
      6-CN
      7-COM.CN
      8.CAN
    
    choice: 4
    
    valid input: 4
    

    If you do your job, you can successfully use scanf as needed.

    Or, you can make your life much easier and use fgets and then test whether the first character in the buffer (by simply dereferencing the pointer) is a valid menu selection, e.g.

    #include <stdio.h>
    
    #define MAXC 1024   /* read buffer max characters */
    
    int main (void) {
    
        int input = 0;
        char buf[MAXC];
        // domainEntry *myDomains = buildDomainDB();
    
        for (;;) {  /* loop continually until valid input or EOF */
            fputs  ("\nSelect top level domain:\n"
                    "  1-EDU\n"
                    "  2-COM\n"
                    "  3-ORG\n"
                    "  4-GOV\n"
                    "  5-MIL\n"
                    "  6-CN\n"
                    "  7-COM.CN\n"
                    "  8.CAN\n\n"
                    "choice: ", stdout);
            if (!fgets (buf, MAXC, stdin)) {
                fputs ("(user canceled input.)\n", stderr);
                return 1;
            }
    
            if (*buf < '1' || '8' < *buf) { /* check 1st char, validate range */
                fputs (" error: invalid input\n", stderr);
                continue;
            }
    
            input = *buf - '0';     /* convert char to integer */
            break;
        }
    
        printf ("\nvalid input: %d\n", input); 
    }
    

    Of course if a key gets stuck and the user enters more than 1023 characters -- characters will remain in the input buffer. But a simple test of whether the last character is '\n' and if not whether MAXC - 1 characters were read will let you know whether that is the case. Your choice, but fgets provides a much easier implementation. Just remember -- don't skimp on buffer size. I'd rather have a buffer 10,000 bytes too long that 1-byte too short....