4

I want a menu from which you choose some action.

Problem is that when we choose one, and press the "return" key, the user input command which should have been the next step, is skipped. Why is that ?

The code is : 

#include <stdio.h>
#include <string.h>

int main(int argc, char *argv[])
{
    int choice;

    do
    {
     printf("Menu\n\n");
     printf("1. Do this\n");
     printf("2. Do that\n");
     printf("3. Leave\n");
     scanf("%d",&choice);

     switch (choice)
     {
        case 1:
            do_this();
            break;
        case 2:
            // do_that();
            break;
     }

    } while (choice != 3);

    return(0);
}

int do_this()
{
    char name[31];

    printf("Please enter a name (within 30 char) : \n");
    gets(name); // I know using gets is bad but I'm just using it

    // fgets(name,31,stdin); // gives the same problem by the way.

    // Problem is : user input gets skiped to the next statement :
    printf("Something else \n");

    return(0);
}
Andriy Ivaneyko
  • 20,639
  • 6
  • 60
  • 82
lapin
  • 2,098
  • 2
  • 21
  • 30
  • What are you doing with the user input? – Magisch Jan 20 '16 at 13:18
  • 4
    I think the problem is with the `scanf`. Doesn't that leave the newline in `stdin`? Maybe that's what's skipping the `fgets`. However, I don't usually use `scanf` so I can't say for sure. – Arc676 Jan 20 '16 at 13:19
  • You just said the issue. You press two keys for the `scanf`. The number is consumed by the `scanf` while the `\n` (enter) is left in the `stdin`. So, `gets`/`fgets` sees the `\n` and consumes it, thus not waiting for further input. Fix it by using `scanf("%d%*c",&choice);` instead of `scanf("%d",&choice);`. The `%*c` tells `scanf` to scan and discard a character. – Spikatrix Jan 20 '16 at 13:20
  • You could've figured it out by googling the title. There are tons of duplicates out there in the web as well as in stackoverflow. – Spikatrix Jan 20 '16 at 13:26
  • @CoolGuy Your solution works ! And of course I googled it, but didn't find any clear answer : ( – lapin Jan 20 '16 at 13:30
  • See [Why is `gets()` too dangerous to be used?](http://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) for reasons why you should never, ever use `gets()`. – Jonathan Leffler Jan 20 '16 at 13:33
  • Trying to find the right question to duplicate this too. It is such a common problem — but also hard to search for. – Jonathan Leffler Jan 20 '16 at 13:38

2 Answers2

5

scanf() leaves a newline which is consumed by the subsequent call to gets().

Use getchar(); right after scanf() or use a loop to read and discard chars:

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

I know you have commented about gets() being bad. But you shouldn't even attempt to use it even if it's a toy program. It's been removed from the latest C standard (C11) completely and shouldn't be used even if you are programming for C89 (due to its buffer overflow vulnerabilities). Use fgets() which does almost the same except possibly leaves a trailing newline.

If this is your complete code, then you would also need a prototype or at least a declaration for do_this(). Implicit int rule has also been removed from C standard. So add,

int do_this();

at the top of your source file.

P.P
  • 117,907
  • 20
  • 175
  • 238
  • 1
    In C, the prototype should be `int do_this(void);` (or, better, `static int do_this(void);` since the function won't be called from outside this file — if it was going to be called from outside this file, then the declaration should be in a header, of course). The significance of this is that `int do_this();` simply says "there is a function that returns an `int` but the argument list is undefined — but it isn't a variadic function with ellipsis `...` as the last argument". It does not state that the function takes no arguments, unlike in C++. – Jonathan Leffler Jan 20 '16 at 13:43
  • Thank you all for you answers. That was the reason of the problem. I used the quick fix suggested by Cool Guy above. – lapin Jan 20 '16 at 16:47
  • @lapin Which is essentially equivalent to adding a `getchar();` call after scanf() call i.e. read `%*c` (which is called *assignment suppression character* ) reads and discards one char. As long as you understand the underlying problem, you can use any of the solutions that fits your need. – P.P Jan 20 '16 at 16:50
1
scanf("%d",&choice);

This leaves the newline ('\n') in the standard input buffer stdin.

Your call of gets() merely consumes that whitespace, writing nothing into name.

To prevent this, consume the newline char after you call scanf, for instance by using getchar(). If you are not on a microsoft platform, please do not use fflush(stdin), as that is undefined behavior (on non-MS platforms).

Magisch
  • 7,312
  • 9
  • 36
  • 52
  • Any of that would be unnecessary if I could get the user choice as `int` without `scanf`, but is that possible ? – lapin Jan 20 '16 at 13:25
  • @lapin I don't think so, unless you want to jump through hoops to get it as string and hand-convert it to int. This could work, as all decimal numbers are follow-ups of each other in ascii. You could also do a formal check if the user entered the correct input this way. – Magisch Jan 20 '16 at 13:26
  • 1
    @lapin It's not the `scanf()` itself cauing the problem but the combination of `scanf()` + `[f]gets()` that's causing the problem. In general, it's better not to mix `scanf()` and `fgets()` and even better would be to not use `scanf()` at all. A more robust approach would be to use fgets() and then `sscanf()` on the line. – P.P Jan 20 '16 at 13:32
  • Note that on Microsoft platforms, [Using `fflush(stdin)`](http://stackoverflow.com/questions/2979209/using-fflushstdin) is defined behaviour. – Jonathan Leffler Jan 20 '16 at 13:44
  • @JonathanLeffler Thats exactly what I wrote in my answer. Read it again, please. – Magisch Jan 20 '16 at 13:45
  • 1
    AFAICS, you wrote "Using `fflush(stdin)` is undefined behaviour" unconditionally — I am commenting that on Microsoft, `fflush(stdin)` has defined (and useful) behaviour, so your sweeping unconditional statement is too sweeping because it is unconditional, and it is particularly a problem because many of the people who use it are working in an environment where it is defined behaviour. These are two different descriptions of the utility of `fflush(stdin)`. – Jonathan Leffler Jan 20 '16 at 13:49
  • @JonathanLeffler Alright fair enough, I'll edit that in. – Magisch Jan 20 '16 at 13:50