1
\$\begingroup\$

While translating an old Basic game to C, I found myself needing a function to get one character from the keyboard. You can't do this with common standard C library functions like getchar() because the standard input stream is line-buffered (i.e. it will store whole line of input, including the terminating \n, in its internal buffer although getchar() only uses its first character. Subsequent calls to getchar() will consume buffer's remaining characters until exhausted and only then will it resume accepting user's new inputs). This causes problems because if you need inputting twice, the second time you will get some unexpected value instead of letting user entering second character.

Now, I know the way around this is to use operating system calls such as ioctl and read to set input to an unbuffered state and read a character (or better yet use a library like Curses which abstracts away all this in a cross-platform way.) but I started wondering if it was possible to do this entirely via the standard C library.

My first attempt was to empty out the remaining characters in stdin's buffer by adding a call to:

fflush(stdin);

...after calling getchar() but this didn't do anything.

My next attempt was to try and take stdin out of line buffer mode like this:

setvbuf(stdin, NULL, _IONBF, 0);

I had high hopes this would work but actually it seems calling setvbuf on stdin is undefined behavior. It certainly does not work on Linux.

So finally I came up with this function. It works but I have a nagging feeling it could be improved. What do you think?

int getkey(const char* prompt = "") {
    /* Print the prompt message if there is one */
    if (strcmp(prompt, "") != 0) {
        puts(prompt);
    }

    /* Get a character and examine it.  If it is a newline from a previous
       call to this function eat it otherwise put it back in the buffer. */
    int c = getchar();
    if (c != '\n') {
        ungetc(c, stdin);
    }

    /* This is the character we really want. */
    c = getchar();

    /* Drain the input buffer so any extra characters which were pressed are
       discarded except for newline which is needed to actually send the input 
       to stdin. */
    int next;
    while(!feof(stdin) && next != '\n') {  
        next = getchar();
    }

    return c;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Are you sure it's valid C ? \$\endgroup\$ – Calak Nov 23 '18 at 8:42
2
\$\begingroup\$

Your strcmp can be replaced with:

if (*prompt) {

Your last loop has issues. You effectively have both a precondition (feof) and a postcondition (next). You can replace the lot with:

while (!feof(stdin))
    if (getchar() == '\n')
        break;
\$\endgroup\$
  • 1
    \$\begingroup\$ I'm surprised you didn't mention the UB when next is used uninitialized... \$\endgroup\$ – Toby Speight Nov 23 '18 at 9:21
  • \$\begingroup\$ Wait, next won't default initialize to 0? Or am I thinking of C++ again? \$\endgroup\$ – Jaldhar Nov 23 '18 at 11:27
  • \$\begingroup\$ @TobySpeight I would have mentioned it, but I decided to omit the variable entirely instead \$\endgroup\$ – Reinderien Nov 23 '18 at 15:39
  • \$\begingroup\$ @Jaldhar Why do you think next is initialized in C++? \$\endgroup\$ – chux Nov 24 '18 at 5:57
2
\$\begingroup\$

if it was possible to do this entirely via the standard C library.

Yes, yet OP's code has issues.

Invalid C

Standard C does not have default function parameters. Even if it did, as in C++, the usage belongs in a .h file declaration, not the .c file definition.

// int getkey(const char* prompt = "") {
int getkey(const char* prompt) {

No need to involve stdout in a stdin function

Consider dropping the prompt code. If still desired, code as a higher level function. Also allow a prompt without an output '\n' occurring. Be prepared for stdout as fully buffered or unbuffered.

  int prompt_and_getkey(const char* prompt) {
    // Do not use puts() which appends a \n, let caller decide.
    fputs(prompt, stdout);
    fflush(stdout);  // Ensure output occurs before input.

    return getkey();
  }

Bug

Below code uses next before it is assigned, thus undefined behavior (UB) and anything may happen. This also implies that OP does not have all warnings enabled with a good compiler as such trivial errors are automatically detected. Save time. Enable all warnings.

int next;
while(!feof(stdin) && next != '\n') {    // bad code

Handle rare input error

feof(stdin) is simply the wrong test as the below is an infinite loop on input error. The earlier answers are also infinite loops on input error.

// bad
int next;
while(!feof(stdin) && next != '\n') {  
    next = getchar();
}

// Amended
int next = ch;
while(next != EOF && next != '\n') {  
    next = getchar();
}

Questionable design

Code does not consider that the input may be simple "\n". Code assumes a first read '\n' is due to a previous line. Instead it may simple be a line that only consists of "\n"

To fix, code needs to re-architect the whole idea of leaving '\n' in stdin for the next line to read. Better to have code finish consuming the line before calling code to get the next line.

fflush(stdin) and feof()

Calling these functions are often strong indications of questionably-designed code. I recommend to never use fflush(stdin) and then use feof() only once one is very clear on its need: to distinguish end-of-file from input error. Do not use feof() to determine if end-of-file occurred. Use ... == EOF to determine if end-of-file or input error occurred.

Unclear comments

Example: "Drain the input buffer so any extra characters which were pressed are discarded except for newline". The except does not apply here as code also discards '\n'.

Simplification

Code only needs to check the first character. Testing *string_pointer for an empty string test is idiomatic in C and is DRY.

// WET
// if (strcmp(prompt, "") != 0) {

// DRY
if (*prompt) {

Either source code may emit the same run-time code. As with such style issue, consult your group's coding guide.

\$\endgroup\$
1
\$\begingroup\$

First, I'm not sure, but default parameter isn't valid in C

Otherwise, you lack includes:

#include <stdio.h> //puts, getchar, printf
#include <string.h> //strcmp

You can get rig of strcmp call (and by extension, the string.h header):

if (prompt != NULL && *prompt) {...}

Finally, your loop can be simplified, removing useless (and unsafe) variable:

 while(!feof(stdin) && getchar() != '\n');
\$\endgroup\$
  • \$\begingroup\$ Oops just noticed I've been compiling this program as C++ (which I normally use) not C. The intent was to write C though. Also: @reinderien made a similar observation about the strcmp but he did not do an explicit NULL check. Is that really necessary? \$\endgroup\$ – Jaldhar Nov 23 '18 at 11:18
  • \$\begingroup\$ This is one function out of a larger program so I didn't bother showing the includes. Of course they are there in the actual codebase. \$\endgroup\$ – Jaldhar Nov 23 '18 at 11:25
  • \$\begingroup\$ For your first message, to make a long explanation short, read this. Otherwise, for the 2nd point, since you have to provide a full working code, you have to provides includes. Take care for the next :) \$\endgroup\$ – Calak Nov 23 '18 at 11:36
  • \$\begingroup\$ @Jaldhar It's not a bad idea, for robustness. \$\endgroup\$ – Reinderien Nov 23 '18 at 15:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.