gets() not working for me

Question

Quick issue with my code. Please look in the function enter(). When I request the name, and use gets(), it just skips it as if nothing was there, and jumps to requesting the age. What am I doing incorrectly?

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

#define RECORDS 30
#define LEN 20

/*Questions
Formatting display() - can we use spaces to format?
Is the patient structure supposed to be global or local in enter()?
Why doesn't printf("name: "); scanf("%s", name); require &name?
Make sure you account for first and last names, use getline or something
*/

void enter();

struct patient
{
    char name[LEN];
    int age;
    double highBP, lowBP, riskFactor;
};

struct patient * db[RECORDS];
int counter = 0;

main()
{
    int flag = 1;

    while (flag == 1)
    {       
        printf("---------------------------------------------------------\n");
        printf("|\t(N)ew record\t(D)isplay db\t(U)pdate record |\n");
        printf("|\t(L)oad disk\t(W)rite disk\t(E)mpty disk    |\n");
        printf("|\t(S)ort db\t(C)lear db\t(Q)uit          |\n");
        printf("---------------------------------------------------------\n");
        printf("choose one: ");

        char selection, selectionstr[3];
        scanf("%s",selectionstr);
        selection = selectionstr[0];        

        //printf("selection %c\n", selection);

        if ((selection == 'n') || (selection == 'N'))
        {
            //New record
            enter();
        }

            // Options omitted...

        else
        {
            printf("not a vaild input\n");  
        }
    }
}

void enter()
{
    if (counter < 30)
    {
        FILE *fptr;
        fptr = fopen("db.txt", "a");

        char name[LEN];
        int age;
        double highBP, lowBP;
        printf("name: "); //scanf("%s", name);
        gets(name);
        printf("age: "); scanf("%d", &age);
        printf("high bp: "); scanf("%lf", &highBP);
        printf("low bp: "); scanf("%lf", &lowBP);
        struct patient temp;
        strcpy(temp.name, name);
        temp.age = age;
        temp.highBP = highBP;
        temp.lowBP = lowBP;

        if ((highBP < 120) && (lowBP < 80))
            temp.riskFactor = 0;

        if (((highBP <= 120) && ((lowBP > 80) && (lowBP <= 90))) || ((highBP <= 80) && ((lowBP > 120) && (lowBP <= 130))))
            temp.riskFactor = 1;

        if (((highBP <= 120) && ((lowBP > 90) && (lowBP <= 100))) || ((highBP <= 80) && ((lowBP > 130) && (lowBP <= 140))))
            temp.riskFactor = 4;

        else
            temp.riskFactor = 5;

        if ((temp.riskFactor > 0) && (temp.age > 50))
            temp.riskFactor += 0.5;

        db[counter] = &temp;
        //fprintf(fptr, "%s, %d, %f, %f, %f\n", db[counter]->name, db[counter]->age, db[counter]->highBP, db[counter]->lowBP, db[counter]->riskFactor);
        printf("%s, %d, %f, %f, %f\n", db[counter]->name, db[counter]->age, db[counter]->highBP, db[counter]->lowBP, db[counter]->riskFactor);
        counter++;

        fclose(fptr);
    }

    else
        printf("database full");


    /*db[counter] = (struct patient *) malloc(sizeof(struct patient));


    printf("name: "); scanf("%s", (*db[counter]).name);
    printf("age: "); scanf("%d", (*db[counter]).age);
    printf("high bp: "); scanf("%d", (*db[counter]).highBP);
    printf("low bp: "); scanf("%d", (*db[counter]).lowBP);


    //db[counter] = &temp;
    printf("%d", sizeof(db[counter]));
    //printf("%s", (db[counter])->name);
    printf("%s, %d, %f, %f\n", db[counter]->name, db[counter]->age, db[counter]->highBP, db[counter]->lowBP);
    counter++;*/
}

Don't use [`gets`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/gets.html). Ever. Use [`fgets`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fgets.html). Only use `gets` if you like buffer overflows and broken software. — mu is too short, Jul 14 '11 at 11:29
Thanks! It's not a duplicate, same code, different question. But thanks anyways! — seyelent, Jul 14 '11 at 12:36

score 5 · Answer 1 · answered Jul 14 '11 at 11:29

5

You do this:

scanf("%s",selectionstr); /* Leaves '\n' in the input buffer. */

And then you do the gets

gets(name); /* Encounters the leftover '\n'. */

The problem is that scanf doesn't consume the newline, which remains in the input buffer. When gets comes along, it encounters the newline and is immediately satisfied.

This question comes up often enough. It's also a C FAQ. And explained quite well here.

There are things to consider:

Someone will propose "flushing stdin". This is a bad idea.
Someone will propose something like
```
while((c = getchar()) != '\n' && c != EOF);
```
which is also troublesome

Your best bet is not to mix up scanf and gets.

As a side note, you do realize you should be using fgets instead of gets, right ?

answered Jul 14 '11 at 11:29

cnicutar

178,505
25
365
392

I proposed the troublesome code :p Why is it troublesome? – Christian Jul 14 '11 at 11:34
@Christian See http://c-faq.com/stdio/gets_flush2.html Of course, it could work out well for you but discarding user input is generally to be avoided. – cnicutar Jul 14 '11 at 11:36
Good point. I guess the function can be misused. But then again, mixing scanf and gets/fgets is a form of misusing those functions, too :D – Christian Jul 14 '11 at 11:40
I forgot we were told in class not to use gets. Thanks, I think it's locked into my brain now :) – seyelent Jul 14 '11 at 12:35

Jonathan Leffler · Accepted Answer · 2011-07-14T11:51:58.130

4

Main Problems

Do not use gets(), ever. Not even in toy programs. It is a bad habit to get into. It cannot be used safely, ultimately, because you cannot tell when the input is going to overflow the buffer you provide.

Use fgets() instead, and remember to remove the trailing newline it leaves behind (the one gets() removes).

Your problem is that the previous input with scanf() didn't read the newline, so gets() reads to the first newline - giving you an empty string. You'd run into the same problem on the next iteration, too, because you read the blood pressure with scanf().

Auxilliary Problems

You do not check that the inputs succeeded; always check the return value from scanf() to ensure that you got the inputs you expected. You'd be getting errors because the name can't be interpreted as a number.

You should get into the habit of writing code that declares functions with prototypes; the notation:

void enter();

says "there is a function enter() that returns no value and takes an indeterminate argument list - any set of arguments is OK, but it isn't a variable argument list function". By contrast:

void enter(void);

says "there is a function enter() that returns no value and takes no arguments".

Also, you should explicitly define the return type of main() as int. It has been more than a decade since the C standard required explicit return types on all function definitions.

Incipient Problem

Once you get over the data scanning problem, you will find that you are not allocating space properly for your records. Your array is:

struct patient *db[RECORDS];

In the loop, you have:

struct patient temp;

db[counter] = &temp;

The troubles are two-fold. You are re-using the same space for each patient record, and the pointer you are storing goes out of scope when enter() exits. The simplest fix is to change the global variable to:

struct patient db[RECORDS];

You can then do away with the temp variable, and simply initialize the appropriate entry in db. Alternatively, you can keep temp but do structure assignment instead of pointer assignment:

db[counter] = temp;

edited Jul 14 '11 at 11:51

answered Jul 14 '11 at 11:31

Jonathan Leffler

730,956
141
904
1,278

Extremely thorough explanation, wow! Much appreciated! Some of the things you mentioned I cannot change as they are instructed to be that way by my professor. – seyelent Jul 14 '11 at 12:36
Actually, one question. How do I remove the trailing new line? – seyelent Jul 14 '11 at 12:40
@seyelent: `buffer[strlen(buffer)-1] = '\0';` as long as you are confident that there is a newline there. If `fgets()` returned you the initial segment of a line that was longer than your buffer, it would zap something else. So, the safe way is: `size_t len = strlen(buffer); if (buffer[len-1] == '\n') buffer[len-1] = '\0';`. – Jonathan Leffler Jul 14 '11 at 13:37
Hmm that makes sense. I also tried another method. Since I am sure there is going to be a new line since the user has to press enter after selecting the option from the menu, I just put a getchar() after it. A blank getchar() seems to soak up the extra \n there, and everything seems to be working fine. Am I making a stupid mistake doing this? – seyelent Jul 14 '11 at 13:54
@seyelent: You need to loop to the newline in case the user types 'new' instead of just 'N' as the command. There is discussion after other answers about whether this is a good idea. It works, but might throw away arbitrary input, but you would probably just diagnose that you don't know what to do with that input anyway. (You also run into problems if the user is a techno-brute who types 'N' and then control-D; this gets the N to the program without sending a newline. Your single `getchar()` would gobble whatever they typed next; your looping `getchar()` would eat the name. This is a paradox.) – Jonathan Leffler Jul 14 '11 at 14:54
These days, I recommend `buffer[strcspn(buffer, "\n")] = '\0';` to neutralize the newline, if it is present. If it's not present, `strcspn()` reports on the length of the string (the number of 'not a newline characters' in the string) and the assignment overwrites the null byte (which is safe). No conditional needed. It is a design defect (of sorts) that `fgets()` doesn't return the length of the data that it read — which means you can't spot when null bytes are included in the data, amongst other problems. It also means you have to rescan the string to find its length. – Jonathan Leffler Aug 02 '17 at 19:21

score 1 · Answer 3 · answered Jul 14 '11 at 11:31

The problem is that after the scanf-call, there's still a "\n" (or whatever newline is on your system) in the input buffer and when you call gets(), it fetches the newline and returns. What you need to do is to clear the input buffer. Like this, e.g.:

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

score 0 · Answer 4 · answered Feb 20 '16 at 10:00

What used to be:

gets(str);

is now replaced by:

int c;
while ( (c = getchar()) != EOF && c != '\n' )
    ;
fgets(str, sizeof(str), stdin);
str[strlen(str) - 1] = '\0';

I hate this but this seems to be the minimal code that does what I need. And this is even assuming that the user hits 'enter' at the end of input. It's not robust!

gets() not working for me

4 Answers4

Main Problems

Auxilliary Problems

Incipient Problem