Properly read integer in a range in C

Question

I wanted to write a C function C99/POSIX compliant that read an integer from the user input. I wanted this function to be safe and robust but I feel it is way too complex for such simple task.

I am wondering whether this code is optimal.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
/**
 * Read an integer from `stdin`.
 * @param min Minimum accepted value
 * @param max Maximum accepted value
 * @param nom Default value
 * @return captured integer
 */
int read_integer(char *text, int min, int max, int nom) {
 int n = nom;
 bool failure = false;
 do {
 printf("%s [%d] ? : ", text, nom);
 // Read user input
 char line[24];
 do {
 if (fgets(line, sizeof(line), stdin) != line || feof(stdin)) {
 exit(EXIT_FAILURE);
 break;
 }
 } while (strchr(line, '\n') == NULL);
 // Default value?
 {
 char *cursor = line;
 while ((*cursor == ' ' || *cursor == '\t') && *cursor != '0円') {
 cursor++;
 } 
 if (*cursor == '\n') {
 return n;
 }
 }
 // Not a number ?
 if (sscanf(line, "%d", &n) != 1) {
 printf("Error: this is not valid entry!\n\n");
 continue;
 } 
 // Not in the range ?
 if (n < min || n > max) {
 printf("Error: value should be between %d and %d!\n\n", min, max);
 continue;
 }
 return n;
 } while(true);
}
int main() {
 do {
 printf("You said %d\n", 
 read_integer("What's the answer", 10, 50, 42)); 
 } while(!feof(stdin));
}

• πάντα ῥεῖ: That code doesn't look to be working like intended.
• nowox: Could you be more explicit?

Answer 1

regarding:

if (fgets(line, sizeof(line), stdin) != line || feof(stdin)) 

This is unnecessary messy. Suggest:

if( fgets( line, sizeof( line ), stdin )

If any error occurs, the returned value is NULL so the body of the if() will not be entered.

regarding:

if (fgets(line, sizeof(line), stdin) != line || feof(stdin)) {
 exit(EXIT_FAILURE);
 break;

the break; will never be executed because the call to exit() will have already exited the program.

regarding:

while(!feof(stdin));

please read: while(!feof()) is always wrong

regarding:

 return n;
} while(true);

The return is always executed, so this loop will never iterate, looking for a valid input.

regarding:

char *cursor = line;
 while ((*cursor == ' ' || *cursor == '\t') && *cursor != '0円') {
 cursor++;
 } 
 if (*cursor == '\n') {
 return n;
 }

This while() code block will iterate to the end of the array line[], most of the time. the result will be no number will be extracted. Suggest, starting at line[0] to check for isdigit( line[i] ) and if true, then extract the number, perhaps using something like: strtol()

• pacmaninbw: We generally prefer not to answer questions that contain code that is not working. I would suggested that you should have picked the part of the code that is not working and comment on that. (return n; } while(true)