Input validation

Question

I am learning C from C Primer Plus, and am still a beginner. I have been introduced to only two functions for input, scanf() and getchar().

According to me, when stdin is associated with input from a keyboard, there are 4 possibilities for user input in this program:

1. User provides valid input and presses enter.
2. User provides valid input and triggers EOF.
3. User provides invalid input (including no input) and presses enter.
4. User provides invalid input (including no input) and triggers EOF.

For (1) and (2), I want the program to continue normally. For (3), I want the program to keep asking the user for valid input. For (4), I want the program to abort.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

#define PROMPT "Enter a non-negative number: "


double get_valid_input(void);
bool flush(void);


int main(void)
{

    double inputNum = get_valid_input();
    printf("Your input = %.2f\n", inputNum);

    return 0;

}


double get_valid_input(void)
{

    double inputNum;
    bool endOfFile;

    int a;
    while ((printf(PROMPT), (a = scanf("%lf", &inputNum)) != 1)
           || ((a == 1) && (inputNum < 0.0)))
    {
        a = 0;
        endOfFile = flush();
        if (endOfFile == true)
            break;
    }

    if (a == 0)
        exit(EXIT_FAILURE);
    else
        endOfFile = flush();

    return inputNum;

}


bool flush(void)
{

    int f;
    while ((f = getchar()) != '\n' && f != EOF)
        continue;

    if (f == EOF)
    {
        printf("\n");
        return true;
    }
    else
        return false;

}

Answer 1

Non-exported functions

Declare get_valid_input and flush as static since you're in a single translation unit.

Foot-cannon one-liners

while ((printf(PROMPT), (a = scanf("%lf", &inputNum)) != 1)

takes the common C antipattern of in-condition assignment even further: it's an entire code-block in a condition. Please consider rewriting this to

while (true) {
    printf(PROMPT);
    a = scanf("%lf", &inputNum);
    if (a == 1 && inputNum >= 0)
        break;
    // ...

scanf

It's a mess. It pollutes the input buffer on failure, among other nasty surprise behaviours. Any serious attempt at comprehensive input validation ends up ditching scanf in favour of sscanf. Read http://c-faq.com/stdio/scanfprobs.html and http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html for flavour.

Answer 2

General Observations

• The code is quite readable.
• Using void in the function declarations is good.
• Using stdlib.h for EXIT_FAILURE is good, but the return from main() should be return EXIT_SUCCESS;

Avoid Global References of Any Form

There are good reasons to use #define CONSTANT to define constants used throughout programs, but doing this for prompt strings probably isn't a good idea. If the program being developed needs to communicate with users in multiple languages it is better to pass the C equivalent of a string char *str into a function instead. It would be better to define prompt as a local variable in main and pass it by reference into the function get_valid_input().

Function Prototypes

In a small program such as this it is better to put the functions in the proper order to reduce the amount of code to be written. Reducing the amount of code necessary is better because if the function declarations need to change they only need to be changed in one place rather than multiple places. Function prototypes are best when linking multiple code files together, then the prototypes are in a header file associated with the source file.

Answer 3

Bug

"User provides ... no input and presses enter.". scanf("%lf", &inputNum) does not return in that case. Waits for non-white-space input.

Beware % in the prompt

printf(PROMPT) can lead to undefined behavior (UB) with when PROMPT has a '%' in it. Use fputs()

// printf(PROMPT)
fputs(PROMPT, stdout)

Bug

"Enter a non-negative number" --> Rejecting values with inputNum < 0.0 still allows input like "-0.0" and "NaN".

inputNum < 0.0 --> (signbit(inputNum) || isnan(inputNum))or the like.

Bug

scanf("%lf", &inputNum) allows an input line like "123.4xyz\n" as scanf("%lf"... does not work with a line of input.

Debug with %g

"%g" is more informative with small values and less noise with large ones.

// printf("Your input = %.2f\n", inputNum);
printf("Your input = %.2g\n", inputNum);

---

Pedantic: stdin error not handled clearly

Code fails to distinguish between end-of-file and rare stdin error. stdin error is different than textual input error.

Pedantic: flush() may consume good data

Rare: scanf("%lf", &inputNum) returns EOF due to a transient input error. The following flush() then may consume good input.

---

Alternative

Some unchecked sample code to explore other ideas.

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

// Reasonable input perhaps as long as DBL_MAX in non-exponential notation.
#define GET_POSITIVE_LINE_N DBL_MAX_10_EXP

double get_postive_double(const char *prompt) {
  // Readily handle inputs up to 2x reasonable length.
  // More code needed to detect nefarious super long inputs.
  char buf[GET_POSITIVE_LINE_N * 2];

  // Possible to construct other loop styles.
  // I find the below clear.
  while (1) {
    if (prompt) {
      fputs(prompt, stdout);
    }

    if (fgets(buf, sizeof buf, stdin) == NULL) {
      // Suggest some note about early exit and where
      fprintf(stderr, "%s:%u end-of-file", __FILE__, __LINE__);
      exit(EXIT_FAILURE);
    }

    // As reasonable, declare objects when needed
    char *endptr;
    errno = 0; // See note below
    double inputNum = strtod(buf, &endptr);

    // No conversion
    if (buf == endptr) {
      continue;
    }

    // Look for trailing non-white-space junk
    while (isspace(*(unsigned char* )endptr)) {
      endptr++;
    }
    if (*endptr) {
      continue;
    }

    // Allow only positive values
    if (signbit(inputNum) || isnan(inputNum)) {
      continue;
    }

    return inputNum;
  }

  // Code never reaches this point
}

strtod() could first use a errno==0 to later detect various errors. I find the default handling sufficient and detailed handling of errno, especially with tiny values not that portable, so left that pedantic corner out.