Why is the following code not allowing me to get user input with fgets yet works with scanf?

user_10431460 0
Viewed 1305

This is a short excerpt from a bigger program, but the rest of the program is irrelevant since I think I was able to isolate the issue. I suspect that it has something to do with the way I'm using fgets. I've read that it's preferable to use fgets over scanf, but I can't seem to get it to work properly here. When I use the following code, the program doesn't give me a chance to enter the number (but simply skips to the while loop which checks if the number entered is in the correct range):

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

#define SIZE 10

int main(void)
{
    // ask user for how many items to store
    printf("how many words would you like to enter? (1-%i): ", SIZE);

    // save number of words user would like to store
    char *input = malloc(sizeof(char));
    fgets(input, 1, stdin);
    // scanf("%c", input);

    int words = atoi(input);

    printf("the number of words is: %i\n", words);
    while (words < 1 || words > SIZE)
    {
        printf("please enter a number between 1 and %i: ", SIZE);
        scanf("%i", &words);
    }
}

Here's the output I get:

~/workspace/extra_stuff/hash_tables/ $ ./test2
how many words would you like to enter? (1-10): the number of words is: 0
please enter a number between 1 and 10:

As you can see, it never let me enter the number, but simply moved on to the next step seemingly assuming I didn't enter anything.

If I change the code as follows, everything works as planned:

#include <stdlib.h>

#define SIZE 10

int main(void)
{
    // ask user for how many items to store
    printf("how many words would you like to enter? (1-%i): ", SIZE);

    // save number of words user would like to store
    char *input = malloc(sizeof(char));
    // fgets(input, 1, stdin);
    scanf("%c", input);

    int words = atoi(input);

    printf("the number of words is: %i\n", words);
    while (words < 1 || words > SIZE)
    {
        printf("please enter a number between 1 and %i: ", SIZE);
        scanf("%i", &words);
    }
}

P.S.: I do realize that if using scanf, I could immediately store the input to an int variable w/o using atoi to convert char to int; however it seems that fgets requires a char *, so that's why I chose this route. Also, I realize that I'm supposed to free(input) later.

Can someone explain this behavior? Thanks.

EDIT:

Thanks to everyone who has replied so far! Some helpful suggestions there, but it looks like I'm having the same issue further in my program. Here's the code excerpt:

// ask for strings
    for (int j = 0; j < words; j++)
    {
        char buffer[4096];
        // fgets(buffer, 40, stdin);

        // name=calloc(NAME_SIZE, sizeof(char));
        // fgets(name, NAME_SIZE, stdin);

        // printf("size of (array[j]->next)->text is: %lu\n", sizeof((array[j]->next)->text));

        printf("please enter string #%i: ", j);

        fgets(buffer, 4096, stdin);

        printf("you've entered: %s", buffer);

        int length = strlen(buffer);
        printf("word length: %i\n", length); 
}

When I run the program, it once again doesn't give me a chance to enter my input when it's supposed to:

please enter string #0: you've entered: 
word length: 1

EDIT #2:

After working through David's answer and referencing other people's comments and other SO threads, I've come up with the following version of the code, which first asks the user for the number of words they'd like to enter (and validates the input) and then asks the user to enter those words (again, validating the input). It seems to be compiling w/o errors and warnings and functioning properly, though I am not 100% sure I've tested all the possible things that could go wrong with the user input, and there are some bits of the code I still don't completely understand (I'll list them below) -- if anyone has time/desire/patience to look through it and tell me if I can still improve something, please let me know. My goal is to use this code in another program that will ask for user input and store the entries in a hash table.

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

#define BUF_SIZE_WORDS 4096
#define BUF_SIZE_NUMBERS 256
#define MAX_WORDS 10

int word_input(int num_words);

void empty_stdin();

int main(void)
{
    int num_words = 0,       /* number of words to enter */
        word_count_check = 0;          /* word count */

    char buffer[BUF_SIZE_NUMBERS] = "";    /* buffer of sufficient size for input */

    for (;;) /* loop continually until valid input of NUMBER OF WORDS USER WANTS TO ENTER or user cancels */
    {
        printf ("how many words would you like to enter? [1-%d]: ", MAX_WORDS);
        // check for cancellation of input
        if (!fgets (buffer, BUF_SIZE_NUMBERS, stdin))
        {
            fputs ("user canceled input\n", stderr);
            return 1;
        }

        // check if user simply hit enter w/o typing anything
        if(buffer[0] == '\n')
        {
            printf("please enter a value\n");
            continue;
        }


        size_t inlength = strlen(buffer);

        // validate length < BUF_SIZE_NUMBERS - 1
        if (inlength >= BUF_SIZE_NUMBERS - 1)
        {
            fputs ("input exceeds allocated buffer size\n", stderr);
            return 2;
        }

        if (inlength && buffer[inlength - 1] == '\n')
        {
            // printf("hurray!\n");
            buffer[--inlength] = 0;
        }
        else if (inlength == BUF_SIZE_NUMBERS - 1) /* the line was too long */
        {
            printf("you've entered too many characters... please stick to a maximum of %i\n", BUF_SIZE_NUMBERS);
            empty_stdin();
            continue;
        }

        // make sure user actually entered a proper int
        if (sscanf (buffer, "%d", &num_words) != 1) /* sscanf is used for conversion */
        {
            fputs ("invalid conversion to int; please provide valid input\n", stderr);
            continue;
        }

        // check if the number entered is out of range
        if (num_words < 1 || num_words > MAX_WORDS)
            fprintf (stderr, "%2d out of valid range.\n", num_words);
        else
            break; /*if the input has been validated, we can now break out of the for loop */
    }

    // call the word_input function and store its return value in word_count_check
    word_count_check = word_input(num_words);

    // check if the number of words processed equals to the number requested by the user
    if(word_count_check == num_words)
    {
        printf("success!\n");
    }
    else
    {
        printf("something went wrong, since word_count_check != num_words...\n");
    }

}

int word_input(int num_words)
{
    int word_count = 0;

    for(;;) /* loop until word_count == num_words is achieved */
    {
        // declare an array for storing input string
        char buffer[BUF_SIZE_WORDS];
        char valid_input[BUF_SIZE_WORDS];

        // prompt user for input
        printf("please enter a string: ");

        // get input and check for CTRL+D
        if (!fgets(buffer, BUF_SIZE_WORDS, stdin))
        {
            fputs ("user canceled input\n", stderr);
            exit(1);
        }

         // check if user simply hit enter w/o typing anything
        if(buffer[0] == '\n')
        {
            printf("please enter a word that's more than 0 characters\n");
            // empty_stdin();
            continue;
        }

        size_t inlength = strlen(buffer);

        // check if user input exceed buffer size
        if (inlength >= BUF_SIZE_WORDS - 1)
        {
            empty_stdin();
            fputs ("input exceeds allocated buffer size, please try again\n", stderr);
            continue;
        }

        // check if the user entered too many characters
        if (inlength == BUF_SIZE_WORDS - 1) /* the line was too long */
        {
            printf("you've entered too many characters... please stick to a maximum of %i\n", BUF_SIZE_WORDS);
            empty_stdin();
            continue;
        }

        if (inlength && buffer[inlength - 1] == '\n')
        {
            buffer[--inlength] = 0;

            // get rid of trailing spaces using sscanf
            sscanf(buffer, "%s", valid_input);

            // figure out the length of the word the user entered
            int word_length = ((int) strlen(valid_input));
            printf("string length: %i\n", word_length);

            // print out the word entered by the user one character at a time
            printf("you've entered: ");
            for (int i = 0; i < word_length; i++)
            {
                printf("%c", valid_input[i]);
            }
            printf("\n");

            // increment word count
            word_count++;
            printf("word_count = %i\n", word_count);

            if (word_count == num_words)
            {
                return word_count;
            }
        }
    }
}

/* helper function to remove any chars left in input buffer */
void empty_stdin()
{
    int c = getchar();
    while (c != '\n' && c != EOF)
        c = getchar();
}

things I don't completely understand yet:

1) 

if (!fgets (buf, MAXC, stdin)) { /* validate ALL user input */ 
    fputs ("(user canceled input)\n", stderr); 
    return 1; 
}

--- does this simply check if the user manually entered EOF (with ctrl+d), or does it check for something else too?

2) calling the empty_stdin() function below seemed to be causing some kind of a weird hang-up where it looked like the program was expecting further input from me as opposed to just going on to the next step, especially when I used it frequently (I figured why not clear the input stream every time the user types in something weird?) and/or when I decreased the buffer to something very small and then purposefully entered too many characters..

void empty_stdin()
{
    int c = getchar();
    while (c != '\n' && c != EOF)
        c = getchar();
}

3) eventually I want to use some of this code to load a dictionary from a text file (instead of user input) and store it in a hash table, and, in another version, in a trie. Besides using isalpha() to make sure we're only storing words that have letters in them, are there any other checks/validations that need to happen when processing the input, beside the ones above? Should any of the above checks be skipped?

fgets scanf user-input c
1 Answers

There is no magic involved in dealing with strings in C -- but you do need to put your accounting hat on... Why? When dealing with input, you have to not only account for the number of characters that you place into your buffer (or wherever you are storing your input), but you also must account for the characters that remain in your input stream!

This is particularly true when using any of the scanf family of function for input. Why? Because on a matching or input failure, processing (reading and removing) characters from your input buffer (stdin here) stops, no further characters are read, and any character causing the matching failure remains unread in your input stream, just waiting to bite you again on your next attempted read.

Compounding this bewilderment for new C programmers is the fact that some conversion specifiers consume leading whitespace (e.g. space, tab, newline,...) and others do not. Your numeric conversion specifiers (along with "%s") consume leading whitespace while "%c" and "%[...]" do not.

All of which are the primary reasons new C programmers are encouraged to use line-oriented input functions like fgets or POSIX getline to handle user input (because they read the entire line at a time -- including the trialing '\n') freeing the new programmer for having to account for ending whitespace or offending characters not converted in the event of a matching failure...

Using fgets followed by a sscanf provides the additional benefit of allowing separate validation of (1) the read of input; and (2) the parse and conversion of input into the needed values.

(note: the only caveat with line-oriented input functions is that they read and include the trailing '\n' in the buffer they fill -- so you will need to "trim" the trailing whitespace as required. You don't want stray '\n' characters dangling off the end of the strings you are storing.)

That said, there will be times when reading input with the scanf family of functions makes sense. There is nothing wrong with doing so, so long as you validate the return every time and handle all three possible conditions:

  1. the user presses ctrl+d on Linux to generate a manual EOF (ctrl+z on windoze);
  2. you handle the matching or input failure cases, including removing any offending characters from your input buffer before your next attempted read; and finally
  3. you have good input (the return indicates all the conversions anticipated, took place).

There is no magic to any of it, but it does take understanding the possible error conditions and handling each of them on every input.

In your case, let's look at your task of getting the number of words to enter from the user. Here you were attempting to read with fgets (that's good!), but you failed to provide sufficient storage to hold the input. When reading small amount of text from the user, a simple array with automatic storage type is all you need. However, you need to size the buffer accordingly (and do NOT skimp on buffer size).

There is no golden rule, but if I had users entering text to convert to a single number, then I would feel good with a 256 character buffer (which provides more than enough to hold the input of any valid number, plus another 230 some-odd characters to handle the time the cat steps on the keyboard, etc..)

For example, taking input from the user and getting the number of words to enter could be done in a manner similar to the following:

#include <stdio.h>

#define SIZE  10    /* good form defining a constant! */
#define MAXC 256    /* max characters for buffer */

int main (void) {

    int nwords = 0,         /* number of words to enter */
        words = 0,          /* each word */
        wc = 0;             /* word count */
    char buf[MAXC] = "";    /* buffer of sufficient size for input */

    for (;;) {  /* loop continually until valid input or user cancels */
        printf ("number of words to enter? [1-%d]: ", SIZE);
        if (!fgets (buf, MAXC, stdin)) {    /* validate ALL user input */
            fputs ("(user canceled input)\n", stderr);
            return 1;
        }
        /* validate length < MAXC - 1 and buf[length-1] == '\n' here */

        if (sscanf (buf, "%d", &nwords) != 1) { /* sscanf for conversion */
            fputs ("  error: invalid conversion to int.\n", stderr);
            continue;
        }
        if (nwords < 1 || SIZE < nwords)  /* validate nwords in range */
            fprintf (stderr, " %2d out of valid range.\n", nwords);
        else  /* good input received, break loop */
            break;
    }

(note: your while loop has been converted to a loop that will loop continually until valid input between 1 < value < SIZE is entered. The condition simply causes control to break; the loop at the point good input is received)

This loop presents a classic fgets/sscanf read and parse of information from the line of input entered by the user. You can parse the number from the line any way you like (but don't use atoi() -- it provides absolutely zero error checking of the conversion). You can use strtol (with proper validation) and you can simply use a pointer to walk-down-the-buffer, picking out digits, converting them from their ASCII to numeric value and then multiplying by 10 and adding as your go. Any way is fine so long as you validate, validate, validate each part of the operation.

Now turning to reading each of the words the user is supposed to enter, we will ignore conventional wisdom and use scanf for the task, but we will handle all three possible cases of the return every time. We will also add a counter to keep track of the valid inputs provided by the user and only exit the loop when we have that number of valid integers provided (or the user cancels by generating a manual EOF).

    printf ("\nnumber of words entered: %d\n", nwords);
    for (; wc < nwords;) {  /* loop continually  */
        int rtn = 0;    /* scanf return */
        printf ("please enter a number between 1 and %d: ", SIZE);
        rtn = scanf ("%d", &words);     /* valdate ALL user input */
        if (rtn == EOF) {           /* handle EOF (manual) */
            fputs ("(user canceled input)\n", stderr);
            break;
        }
        else if (rtn == 0) {    /* handle "matching failure" */
            int c = getchar();  /* remove offending chars from stdin */
            while (c != '\n' && c != EOF)
                c = getchar();
            fputs ("  error: invalid integer input\n", stderr);
            continue;
        }
        else {  /* valid integer received */
            int c = getchar();      /* remove any extra chars from stdin */
            while (c != '\n' && c != EOF)
                c = getchar();
            if (words < 1 || SIZE < words)  /* validate in-range */
                fprintf (stderr, " %2d - invalid! (1 < valid < %d)\n", 
                        words, SIZE);
            else    /* good input, increment word count */
                printf (" word[%2d]: %3d\n", ++wc, words);
        }
    }

Note: the emptying of any offending characters from stdin can be turned into a convenient function so that you do not have to duplicate the loops each time you need to clear stdin during your input routine. You can replace it with a simple function, e.g.

/* helper function to remove any chars left in input buffer */
void empty_stdin()
{
    int c = getchar();
    while (c != '\n' && c != EOF)
        c = getchar();
}

That will help keep your code tidy. I'll let you incorporate that above.

Putting it altogether, you could do something like the following:

#include <stdio.h>

#define SIZE  10    /* good form defining a constant! */
#define MAXC 256    /* max characters for buffer */

int main (void) {

    int nwords = 0,         /* number of words to enter */
        words = 0,          /* each word */
        wc = 0;             /* word count */
    char buf[MAXC] = "";    /* buffer of sufficient size for input */

    for (;;) {  /* loop continually until valid input or user cancels */
        printf ("number of words to enter? [1-%d]: ", SIZE);
        if (!fgets (buf, MAXC, stdin)) {    /* validate ALL user input */
            fputs ("(user canceled input)\n", stderr);
            return 1;
        }
        /* validate length < MAXC - 1 and buf[length-1] == '\n' here */

        if (sscanf (buf, "%d", &nwords) != 1) { /* sscanf for conversion */
            fputs ("  error: invalid conversion to int.\n", stderr);
            continue;
        }
        if (nwords < 1 || SIZE < nwords)
            fprintf (stderr, " %2d out of valid range.\n", nwords);
        else 
            break;
    }

    printf ("\nnumber of words entered: %d\n", nwords);
    for (; wc < nwords;) {  /* loop continually  */
        int rtn = 0;    /* scanf return */
        printf ("please enter a number between 1 and %d: ", SIZE);
        rtn = scanf ("%d", &words);     /* valdate ALL user input */
        if (rtn == EOF) {           /* handle EOF (manual) */
            fputs ("(user canceled input)\n", stderr);
            break;
        }
        else if (rtn == 0) {    /* handle "matching failure" */
            int c = getchar();  /* remove offending chars from stdin */
            while (c != '\n' && c != EOF)
                c = getchar();
            fputs ("  error: invalid integer input\n", stderr);
            continue;
        }
        else {  /* valid integer received */
            int c = getchar();      /* remove any extra chars from stdin */
            while (c != '\n' && c != EOF)
                c = getchar();
            if (words < 1 || SIZE < words)  /* validate in-range */
                fprintf (stderr, " %2d - invalid! (1 < valid < %d)\n", 
                        words, SIZE);
            else    /* good input, increment word count */
                printf (" word[%2d]: %3d\n", ++wc, words);
        }
    }
}

Example Use/Output

$ ./bin/getintstdin
number of words to enter? [1-10]: five, maybe six?
  error: invalid conversion to int.
number of words to enter? [1-10]: -2
 -2 out of valid range.
number of words to enter? [1-10]: 3

number of words entered: 3
please enter a number between 1 and 10: two? three?
  error: invalid integer input
please enter a number between 1 and 10: 2
 word[ 1]:   2
please enter a number between 1 and 10: -2
 -2 - invalid! (1 < valid < 10)
please enter a number between 1 and 10: 11
 11 - invalid! (1 < valid < 10)
please enter a number between 1 and 10: 3
 word[ 2]:   3
please enter a number between 1 and 10: 4
 word[ 3]:   4

Note all the invalid inputs above and how the code handles each. So long as the input does not exceed 255-characters with fgets, the code will gracefully respond to inputs that are not valid integers (regardless how many are given) and it will respond to integer inputs that are out-of-range.

The code isn't much longer than the code you posted, but it addresses the possible error conditions that could arise and then handles the errors. When you boil it all down, that is what coding is all about. Look things over and let me know if you have further questions.

user_34221020
Related