Reading input from stdin

I read user input from stdin in plain C.
The problem is that I want a sane implementation that is robust to errors and restricts the user to a certain input and doesn't suck in terms of complexity.
The function get_strings() reads input char by char as long there is no new line (\n), no EOF and all chars are passing the isalpha() test. But I want to keep spaces.

Some points that (I think) deserve special attention during the review:

• I would love to get rid of the outer while loop which is basically testing wether the user just pressed Enter without any meaningful input.

- Do I even need to use ferror()? fgetc already returns EOF when something did go wrong, but I will just stop reading from the stream instead of telling the user something went wrong.

• It doesn't happen everytime but it happens: A \n stays in the stdin stream and the next time I want to get meaningful input fgetc() just gets skipped. It doesn't matter here, but it does when I am asking single character Yes/No questions. I can only get rid of this problem with a construct that clears out all previous things that are left in stdin. See the second codeblock for this.

So, am I handling this totally wrong? Are there better practices to embrace? It looks really clunky to me, and clunky is always bad.

```
/** @brief Contains the dictionary */
static char **strings = NULL;

/** @brief Helps with containing the dicionary */
static char *string;

/ Reads input char by char with fgetc() /
static char *get_strings()
{
char *string = NULL;
char ch;
size_t len = 0;
while (string == NULL && ch != EOF) {
while (EOF != (ch = fgetc(in_stream)) && ch != '\n') {
if (ch != ' ' && isalpha((int)ch) == 0) {
fprintf(stderr, "Only [a-z] is a valid input. | \t"
"| Input another or end with CTRL+D: ");
continue;
}
string = (char*) realloc(string, len+2);
if (string == NULL) {

Architecture

stdin is usually line buffered. So nothing is given to fgetc() until the user hits Enter. OP code will give multiple error messages with input like "Hello 123". Better to separate user input from input validation. Read the line of user input with fgets() or some version of your own as fgets() does have some weaknesses. Then validate the input.

char *input;
while ((input = my_gets()) != NULL) {
  if (valid_phrase(input)) {
    foo(input);
  } else {
    fprintf(stderr, "Invalid input\n");
  }
  free(input);
}

Concerning "I would love to get rid of the outer while loop". That loop exists to silently consume '\n'. If you want a loop to do that, just preceded inner loop with

int ch;
while ((ch = fgetc()) == '\n')
  ;
ungetc(ch, stdin);

char ch

The ch is not the best type. fgetc() returns typically 257 different values [0-255] and EOF. To properly distinguish them, save the result in an int.

// bad
char ch;
..
while (string == NULL && ch != EOF) {    
while (EOF != (ch = fgetc(in_stream)) && ch != '\n') {

// better
int ch;
..
while (string == NULL && ch != EOF) {
  while (EOF != (ch = fgetc(in_stream)) && ch != '\n') {

Same for char tmp;

realloc()

Cast not need.

Modify for out of memory to free string - not needed if code will simply exits, yet good practice to put your toys (code's pointer) away.

// string  = (char*) realloc(string, len+2);
char * new_string  = realloc(string, len+2);
if (new_string == NULL) {
  free(string);
  bail_out(EXIT_FAILURE, "Out of memory");
}
string = new_string;

Good use of of sizeof(*strings) below. Recommend simplification.

strings = (char**) realloc(strings, (index+1)*sizeof(*strings));
strings = realloc(strings, sizeof *strings * (index+1));

size_t len

Good use of size_t to represent an array size. Curiously code does not do the same with int index;. Recommend size_t index;

is...()

With using int ch, no need for cast. Since this is a logical test, recommend using ! rather than arithmetic == 0.

// if (ch != ' ' && isalpha((int)ch) == 0) {
if (ch != ' ' && !isalpha(ch)) {

The following may be easier to understand - fewer negations. (Style issue)

if (!(ch == ' ' || isalpha(ch))) {

ferror()

Nice check of if (ferror(in_stream))

Variable names

string, strings is as useful as calling an integer integer. Maybe phrase, dictionary instead.

// OK
/** @brief Contains the dictionary */
static char **strings = NULL;

// Better
// comment not truly needed
static char **dictionary = NULL;

get_strings() is mis-named. It sounds general, yet code restricts input to letters and space. Maybe get_words()?

StackExchange Code Review Q#116342, answer score: 5