I have a scanf function that takes an integer 1,2, or 0. My default statement will return to a loop if there is not an input of these integers. If a character is entered, the default case still works, displays an error and returns to main. My question is if I should be using another while loop to check the scanf function for an integer, or if it is okay to keep the default statement which will return for any invalid input.

int main(int argc, char* argv[]) {
    int value;
    value = length_orWeight();
    while (value != 0) {
        value = length_orWeight();
    }
    return 0;
}

int length_orWeight(void) {
    int choice;
    printf("\nWhat would you like to convert?\n0.End Program 1.Lengths 2.Weights: ");
    scanf("%d", &choice);
    clear_keyboard_buffer();
    switch (choice) {
        case 1:
            convert_lengths();
            return 1;
        case 2:
            convert_weights();
            return 2;
        case 0:
            return 0;
        default:
            printf("\nError: You must enter 0, 1, or 2.\n");
            return 3;
    }
}

CodePudding user response：

Not checking the return value of scanf is inadequate.

The C standard does not specify what happens to objects that are not assigned values because input terminated early (as when a non-matching character was found, an end-of-file was encountered, or an input error occurred). A common behavior may be for scanf to leave the object (choice) unchanged. If this occurs in your first execution of scanf, it will remain uninitialized. However, as the standard does not define what scanf does in this regard, other behaviors are possible. For example, a rudimentary scanf implementation could, upon starting to work on %d, initialize choice to zero, in anticipation of building a number in it digit-by-digit. When it then reads a character and finds some non-digit the user entered, it could terminate with a return value indicating no conversion was performed, with choice still set to zero.

If scanf does not return a value that indicates the a conversion was completed that assigned a value to an object you passed it, you do not know what value is in that object, even if you initialized the object prior to calling scanf. Therefore, before using the object at all, you must check the return value of scanf.

CodePudding user response：

Eric Postpischil clearly stated that checking scanf's return value is necessary, but didn't point to the fact that it is a very bad function for reading input from the user. Better use fgets and sscanf:

// Reads one line
char *scanline(char *line, size_t size, FILE *istream)
{
    if (!fgets(line, size, istream)) // Input failed
        return NULL;
    
    // Replace newline with null-terminator
    size_t nl = strcspn(line, "\n");
    line[nl] = '\0';
    
    return line;
}

// Reads an int
bool scanint(int *i, FILE* istream)
{
    char buffer[255];
    if (!scanline(buffer, sizeof buffer, istream))
        return false;
    
    if (sscanf(buffer, "%d", i) != 1)
        return false;
    
    return true;
}

Also, make sure that clear_keyboard_buffer() is not fflush(stdin):

void clear_keyboard_buffer(void)
{
    scanf("%*[^\n]"); // Read and discard everything until hitting a newline.
}

Here is how your code would look like:

int length_orWeight(void)
{
    int choice;
    printf("\nWhat would you like to convert?\n0.End Program 1.Lengths 2.Weights: ");
    
    while (!scanint(&choice, stdin)) {
        printf("Please enter a valid number: ");
    };
    
    // clear_keyboard_buffer(); // No longer necessary
    switch (choice) {
        case 1:
            //convert_lengths();
            return 1;
        case 2:
            //convert_weights();
            return 2;
        case 0:
            return 0;
        default:
            printf("\nError: You must enter 0, 1, or 2.\n");
            return 3;
    }
}

int main(int argc, char* argv[])
{
    int value;
    value = length_orWeight();
    while (value != 0) {
        value = length_orWeight();
    }
}