0

I recently tried to use fgets() instead of scanf() to read a string for code security reasons. I used a simple function that I found here to check for errors (no input and too long input). The problem is that whenever i press "ENTER" without actually writing anything, fgets() doesn't return NULL and my program is not able to show the NO_INPUT error.

Here's main.cpp:

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

#include "utilities.h"

int main() {
    int rc;
    char str[20];

    rc = getLine("Enter string: ", str, sizeof(str));
    if(rc == NO_INPUT) {
        printf("[ERROR] No input\n\n");
    } else if(rc == TOO_LONG) {
        printf("[ERROR] Input is too long\n\n");
    } else {
        printf("This is your input: \"%s\"\n\n", str);
    }

    system("pause");
}

Here's utilities.h:

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

#define OK       0
#define NO_INPUT 1
#define TOO_LONG 2

int getLine(const char *msg, char *buff, size_t len) {  
    if(msg != NULL) {
        printf("%s", msg);
        fflush(stdout);
    }

    if(fgets(buff, len, stdin) == NULL /*[+]*/ || strcmp(buff, "\n") == 0) {
        //[-] fflush(stdin);
        return NO_INPUT;
    } else if(buff[strlen(buff)-1] != '\n') {
        //[-] fflush(stdin);
        return TOO_LONG;
    } else {
        buff[strlen(buff)-1] = '\0';
        //[-] fflush(stdin);
        return OK;
    }
}

And here's the output:

Enter string:
This is your input: ""

Press any key to continue . . . 

I solved my problem by replacing the first if statement in utilities.h with if(fgets(buff, len, stdin) == NULL || strcmp(buff, "\n") == 0). Doing this, my program will check for input errors OR for empty strings and return the NO_INPUT flag.


Thanks to everybody who commented and @Peter for answering. I added the aforementioned if statement in utilities.h and removed every fflush(stdin) occurrence. The code now looks as above.

sgrontflix
  • 91
  • 1
  • 9
  • 4
    When you just press ENTER, `fgets()` *does* read a string... with just a newline in it. It may be a blank line, but there's still a line to read. It would return NULL if the stream were closed or an error occurred, though. – Dmitri Oct 23 '16 at 09:56
  • Well... before I print the string I make sure to replace the '\n' character with a '\0', however if I compare buff[0] with '\0' the output is still the same... Also, I read the manual and it says `If the end-of-file is encountered while attempting to read a character, the eof indicator is set (feof). If this happens before any characters could be read, the pointer returned is a null pointer (and the contents of str remain unchanged). If a read error occurs, the error indicator (ferror) is set and a null pointer is also returned (but the contents pointed by str may have changed).` – sgrontflix Oct 23 '16 at 10:05
  • So, shouldn't I get `NULL` because the '\n' character was encountered before any other character could be read? – sgrontflix Oct 23 '16 at 10:08
  • 1
    `'\n'` isn't end of file... it's just a newline character... and it *is* read by `fgets()`. End of file would happen if the input source were closed and there was nothing left to read (not just nothing *right now* or nothing on that line). – Dmitri Oct 23 '16 at 10:15
  • 2
    Voting to reopen as the duplicate is clearly wrong. – hyde Oct 23 '16 at 10:19
  • @Dmitri I didn't know it... I read somewhere that `\n` is treated like `eof`. Anyway, have you got any ideas on how to fix my problem? Maybe I could compare the input string with the `\n` character to check if it's "empty", what do you think? @hyde What do you mean? I'm new around here : / – sgrontflix Oct 23 '16 at 11:11
  • `fflush(stdin)` is undefined behavior. http://stackoverflow.com/questions/2979209/using-fflushstdin – Andrew Henle Oct 23 '16 at 12:56
  • 1
    @sgrontflix Wherever you read that `\n` is treated like EOF is wrong, if you really read it anywhere, which doesn't seem likely. – user207421 Oct 23 '16 at 23:03
  • @EJP http://digilander.libero.it/uzappi/C/librerie/funzioni/fgets.html I read it from here. Let me translate for you: `fgets() reads a line from the stream and puts it in the buffer pointed by s. At most, (size - 1) characters are read, or until it is reached the new-line '\n' character or the EOF.` I guess I shouldn't have trusted this crappy website... Thank you all for the help. – sgrontflix Oct 30 '16 at 17:53

1 Answers1

4

Your problem that you "fixed" is believing that a end of line should be treated as end of input.

NULL is an indication from fgets() that it encountered an error or the end of input when reading from the file (or stream). A blank line is neither an error nor a marker of end of input. A human (typing on a keyboard) might choose to interpret a newline as end of input, but a computer program does not - after all, there is nothing stopping a user entering more than one line of input.

Practically, fgets() reads a line and indicates the end of that line with a '\n' character. Let's say, we have a file containing

ABC

DE

FGHIJ

(blank lines interspersed in three lines of text, followed by end of the file).

Let's also that buffer is an array of five char, and that we read that file using consecutive statements of the form fgets(buffer, 5, file).

So what will fgets(buffer, 5, file) do on each call. Well, with 1 representing the first call, 2 representing the second call, etc we will see results of

  1. "ABC\n" stored into buffer;
  2. "\n" stored into buffer; (first blank line)
  3. "DE\n" stored into buffer;
  4. "\n" stored into buffer; (second blank line)
  5. "FGHI" stored into buffer;
  6. "J\n" stored into buffer; and
  7. fgets() returns NULL, and nothing is stored into buffer.

The first six calls will all return &buffer[0] - not NULL - since no error is encountered reading from the file. Even though there are two blank lines in the input. The last line, which is longer than the buffer (with the '\n' counted) is read in two parts.

Incidentally, your code is using fflush(stdin). Unfortunately, fflush() only has defined behaviour on OUTPUT streams or files. Using it on stdin (or any input stream) gives undefined behaviour. If it is actually discarding input (which it does with some implementations of the C standard library), you are getting lucky - there are real-world compilers where the resultant behaviour does not discard input.

Peter
  • 35,646
  • 4
  • 32
  • 74
  • 1
    `fgets(buffer, 4, file)` will read (up to) *three* characters from the stream into `buffer`, and then add a null character... not *four* (so the first read would be "ABC" without the newline... the next read, still before the blank line, being "\n"). – Dmitri Oct 24 '16 at 00:09
  • @Dimitri - correct, thanks for that ... edited and fixed. Changed the buffer length, and adjusted accordingly (I didn't want multiple cases of a split line). – Peter Oct 24 '16 at 00:34
  • Thank you very much for clarifying. I have removed fflush(stdin) as suggested. – sgrontflix Oct 30 '16 at 17:55