How to use fgets() to safely handle user input more than once

Question

I'm sorry if I duplicate, but I have tried EVERYTHING, and I can't figure out why this code keeps breaking. The highest-priority goal was to make this code handle input safely, or just anything that the user can type into the console, without it breaking. However, I also need it to be able to run more than once. fgets() won't let me do that since it keeps reading '\n' somewhere and preventing me from entering input more than once when it hits the end of the do/while loop. I have tried fflushing stdin, I have tried scanf("%d *[^\n]"); and just regular scanf("%d *[^\n]");, but none of those work, and, in fact, they break the code! I used this website to try to get the "Safely handling input" code to work, but I don't completely understand what they're doing. I tried to jerry-rig (spelling?) it as best I could, but I'm not sure if I did it right. Did I miss something? I didn't think a problem this seemingly simple could be so much of a headache! >_<

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

using namespace std;

#define BUF_LEN 100
#define SPACE 32
#define SPCL_CHAR1F 33
#define SPCL_CHAR1L 47
#define SPCL_CHAR2F 58
#define SPCL_CHAR2L 64
#define SPCL_CHAR3F 91
#define SPCL_CHAR3L 96
#define NUMF  48
#define NUML 57
#define UC_CHARF 65
#define UC_CHARL 90
#define LC_CHARF 97
#define LC_CHARL 122

void main ()
{
    char* buffer;
    int SpcCounter=0, SpclCounter=0, NumCounter=0,LcCounter=0, UcCounter=0;
    char line[BUF_LEN],response[4];
    char*input="";
    bool repeat=false;

    do
    {
        for(int i=0;i<BUF_LEN;i++)
        {
            line[i]=NULL;
        }
        buffer=NULL;
        printf("Enter your mess of characters.\n");

        buffer=fgets(line,BUF_LEN,stdin);

        //To handle going over the buffer limit: BROKEN
        if(buffer!=NULL)
        {
            size_t last=strlen(line)-1;

            if(line[last]=='\n')

                line[last]='\0';
            else
            {
                fscanf(stdin,"%c *[^\n]");
            }
        }


        for(int i=0;i<BUF_LEN;i++)
        {
            char temp=buffer[i];
            if(temp==SPACE||temp==255)
                SpcCounter++;
            else if((temp >= SPCL_CHAR1F && temp <= SPCL_CHAR1L)||/*Special  characters*/
                (temp >= SPCL_CHAR2F && temp <= SPCL_CHAR2L)||
                (temp >= SPCL_CHAR3F && temp <= SPCL_CHAR3L))
                SpclCounter++;
            else if (temp >=NUMF && temp <= NUML)/*Numbers*/
                NumCounter++;
            else if (temp >= UC_CHARF && temp <= UC_CHARL)/*Uppercase letters*/
                UcCounter++;
            else if (temp >= LC_CHARF && temp <= LC_CHARL)/*Lowercase letters*/
                LcCounter++;
        }

        printf("There were %i space%s, %i special character%s, %i number%s, and %i letter%s,\n"
            "consisting of %i uppercase letter%s and %i lowercase.\n",
            SpcCounter,(SpcCounter==1?"":"s"),SpclCounter,(SpclCounter==1?"":"s"), NumCounter,(NumCounter==1?"":"s"),UcCounter+LcCounter,
            (UcCounter+LcCounter==1?"":"s"), UcCounter,(UcCounter==1?"":"s"), LcCounter);
        printf("Would you like to do this again? (yes/no)");

        input=fgets(response,4,stdin);
        /*
        ALL BROKEN
        if(input!=NULL)
        {
            size_t last=strlen(response)-1;

            if(response[last]=='\n')
                response[last]='\0';
            else
            {
                fscanf(stdin,"%*[^\n]");
                fscanf(stdin,"%c");
            }
        }
        */

        //To capitalize the letters
        for(int i=0;i<4;i++)
        {
            char* temp=&response[i];
            if (*temp >= LC_CHARF && *temp <= LC_CHARL)
                *temp=toupper(*temp);//Capitalize it
        }
        //To set repeat: WORKS, BUT WEIRD
        repeat=!strncmp(input,"YES",4);
    }
    while(repeat);
}

http://stackoverflow.com/questions/4023895/how-to-read-string-entered-by-user-in-c/4023921#4023921 — paxdiablo, Jan 06 '14 at 21:24
That was not enough. I had a problem of understanding how each of the different pieces worked as well, not just getting the answer. If you recall from the post, I had the answer, but my understanding of it was lacking, which is why I came here. Also, your reference did not cover the possibility of being able to handle a while loop using the same fgets, which, with your code, does not seem to work. — Skello, Jan 07 '14 at 01:58
To be more precise, I need to know why I need to use fflush on anything, how did using `getchar()` flush anything from the buffer, what does `fflush(stdin)` really do (it's not in my posted code, but I had it after every request for input), and how do I get rid of the stuff that's, for some reason, STILL ON THE BUFFER, even after I flush it, clear the buffers and do the whole `scanf("%*[^\n]")` thing. — Skello, Jan 07 '14 at 02:02
Sage, flushing input streams is undefined behaviour. In addition, your life will be a great deal easier if you don't try to mix line-based and character-based input. That answer I linked to provides everything you need for line based input, heavily tested over decades. — paxdiablo, Jan 07 '14 at 03:07
And it does work with continuous calls, unless you mess about with character based input in between. It was something for you to look over as a possibility while you waited for a more complete answer (which is why it was a comment). — paxdiablo, Jan 07 '14 at 03:09
My apologies for being so blunt. I've been fighting with this problem all day, without any understanding of what I've been doing. All this stuff I've been trying seems to be either useless or counter-acting with each other. On top of that, I'm having a hard time phrasing my question. Do you mind if I just explode for a sec: tell you the entire problem, or just post it in a separate thread? — Skello, Jan 07 '14 at 08:00
let me have a more detailed look at the question tonight (I'm on my way home). I'll see if I can answer in detail. And don't worry about offending me, my skin's thick enough to stop neutrinos :-) — paxdiablo, Jan 07 '14 at 08:18
Sage, I've added an answer that's (hopefully) able to be understood. Have a look at it and feel free to ask any questions in the comments, and I'll try to expand on any areas you feel are deficient. — paxdiablo, Jan 07 '14 at 11:30

score 2 · Accepted Answer · answered Jan 07 '14 at 10:47

For safe, secure user input in C (and in C++ if I'm using C-style strings), I usually revert to an old favorite of mine, the getLine function:

// Use stdio.h and string.h for C.
#include <cstdio>
#include <cstring>

#define OK       0
#define NO_INPUT 1
#define TOO_LONG 2
static int getLine (char *prmpt, char *buff, size_t sz) {
    int ch, extra;

    // Output prompt then get line with buffer overrun protection.
    if (prmpt != NULL) {
        printf ("%s", prmpt);
        fflush (stdout);
    }
    if (fgets (buff, sz, stdin) == NULL)
        return NO_INPUT;

    // If it was too long, there'll be no newline. In that case, we flush
    // to end of line so that excess doesn't affect the next call.
    if (buff[strlen(buff)-1] != '\n') {
        extra = 0;
        while (((ch = getchar()) != '\n') && (ch != EOF))
            extra = 1;
        return (extra == 1) ? TOO_LONG : OK;
    }

    // Otherwise remove newline and give string back to caller.
    buff[strlen(buff)-1] = '\0';
    return OK;
}

This function:

can output a prompt if desired.
uses fgets in a way that avoids buffer overflow.
detects end-of-file during the input.
detects if the line was too long, by detecting lack of newline at the end.
removes the newline if there.
"eats" characters until the next newline to ensure that they're not left in the input stream for the next call to this function.

It's a fairly solid piece of code that's been tested over many years and is a good solution to the problem of user input.

In terms of how you call it for the purposes in your question, I would add something very similar to what you have, but using the getLine function instead of directly calling fgets and fiddling with the results. First some headers and the same definitions:

#include <iostream>
#include <cstdlib>
#include <cctype>

#define BUF_LEN 100
#define SPACE 32
#define SPCL_CHAR1F 33
#define SPCL_CHAR1L 47
#define SPCL_CHAR2F 58
#define SPCL_CHAR2L 64
#define SPCL_CHAR3F 91
#define SPCL_CHAR3L 96
#define NUMF 48
#define NUML 57
#define UC_CHARF 65
#define UC_CHARL 90
#define LC_CHARF 97
#define LC_CHARL 122

Then the first part of main gathering a valid line (using the function) to be evaluated:

int main () {
    int SpcCounter, SpclCounter, NumCounter, LcCounter, UcCounter;
    char line[BUF_LEN], response[4];
    bool repeat = false;

    do {
        SpcCounter = SpclCounter = NumCounter = LcCounter = UcCounter = 0;

        // Get a line until valid.

        int stat = getLine ("\nEnter a line: ", line, BUF_LEN);
        while (stat != OK) {
            // End of file means no more data possible.

            if (stat == NO_INPUT) {
                cout << "\nEnd of file reached.\n";
                return 1;
            }

            // Only other possibility is "Too much data on line", try again.

            stat = getLine ("Input too long.\nEnter a line: ", line, BUF_LEN);
        }

Note that I've changed where the counters are set to zero. Your method had them accumulating values every time through the loop rather than resetting them to zero for each input line. This is followed by your own code which assigns each character to a class:

        for (int i = 0; i < strlen (line); i++) {
            char temp=line[i];
            if(temp==SPACE||temp==255)
                SpcCounter++;
            else if((temp >= SPCL_CHAR1F && temp <= SPCL_CHAR1L)||
                (temp >= SPCL_CHAR2F && temp <= SPCL_CHAR2L)||
                (temp >= SPCL_CHAR3F && temp <= SPCL_CHAR3L))
                SpclCounter++;
            else if (temp >=NUMF && temp <= NUML)
                NumCounter++;
            else if (temp >= UC_CHARF && temp <= UC_CHARL)
                UcCounter++;
            else if (temp >= LC_CHARF && temp <= LC_CHARL)
                LcCounter++;
        }

        printf("There were %i space%s, %i special character%s, "
               "%i number%s, and %i letter%s,\n"
               "consisting of %i uppercase letter%s and "
               "%i lowercase.\n",
            SpcCounter,  (SpcCounter==1?"":"s"),
            SpclCounter, (SpclCounter==1?"":"s"),
            NumCounter, (NumCounter==1?"":"s"),
            UcCounter+LcCounter, (UcCounter+LcCounter==1?"":"s"),
            UcCounter, (UcCounter==1?"":"s"),
            LcCounter);

Then finally, a similar way as above for asking whether user wants to continue.

        // Get a line until valid yes/no, force entry initially.

        *line = 'x';
        while ((*line != 'y') && (*line != 'n')) {
            stat = getLine ("Try another line (yes/no): ", line, BUF_LEN);

            // End of file means no more data possible.

            if (stat == NO_INPUT) {
                cout << "\nEnd of file reached, assuming no.\n";
                strcpy (line, "no");
            }

            // "Too much data on line" means try again.

            if (stat == TOO_LONG) {
                cout << "Line too long.\n";
                *line = 'x';
                continue;
            }

            // Must be okay: first char not 'y' or 'n', try again.

            *line = tolower (*line);
            if ((*line != 'y') && (*line != 'n'))
                cout << "Line doesn't start with y/n.\n";
        }
    } while (*line == 'y');
}

That way, you build up your program logic based on a solid input routine (which hopefully you'll understand as a separate unit).

You could further improve the code by removing the explicit range checks and using proper character classes with cctype(), like isalpha() or isspace(). That would make it more portable (to non-ASCII systems) but I'll leave that as an exercise for later.

A sample run of the program is:

Enter a line: Hello, my name is Pax and I am 927 years old!
There were 10 spaces, 2 special characters, 3 numbers, and 30 letters,
consisting of 3 uppercase letters and 27 lowercase.
Try another line (yes/no): yes

Enter a line: Bye for now
There were 2 spaces, 0 special characters, 0 numbers, and 9 letters,
consisting of 1 uppercase letter and 8 lowercase.
Try another line (yes/no): no

Thanks for the detailed answer. This was very useful. I'll also take a look at those `cctype()`, `isalpha()` and `isspace()` methods, as I didn't know they existed. I just have one question right now: what exactly did `scanf("%*[^\n])` do? I don't understand from I've read on cplusplus.com. — Skello, Jan 07 '14 at 16:37
I've been hunting for a comprehensive line-reading implementation for over half an hour and this is by far the best one — sosmo, Apr 28 '15 at 16:14
C `getline` is a GCC extension. It is not much help on Solaris even with `-std=c99 -D_GNU_SOURCE`. — jww, Apr 12 '19 at 23:38
@jww, the `getLine` I refer to (note the capital `L`) is my own, and provided in the answer. It bears no relationship (other than a similar name) to the `getline` function provided by gcc (or glibc). It should work on any standard-compliant compiler. — paxdiablo, Apr 13 '19 at 07:32

How to use fgets() to safely handle user input more than once

1 Answers1

Linked