0

Here's the basic skeleton of my code:

int getSentence(SearchResults* input){
    printf("Enter sentence:");
    fgets(input->sentence, 100, stdin);

    printf("Enter message ID:");
    fgets(input->messageID, 20, stdin);
    
    return 1;
}

As is, this has a couple of problems. One is that when I print the string later, they have a newline character at the end, which I don't want. The other is that if I enter more than the 100 or 20 characters, it will overflow those characters into the next fgets call.

I've found "solutions" online, but they all either introduce new problems, or are recommended against using. Most of what I found is summarized in this SO post: How to properly flush stdin in fgets loop.

  1. fflush works for the second problem, but is apparently "problematic".
  2. I came up with *(strstr(input->sentence, "\n")) = '\0'; for the first issue, but it doesn't help with the second
  3. while ((getchar()) != '\n')(and variations) is a commonly recommended solution for both problems, but it introduces a new issue: my next input requires two enters before the program will continue running. I tried adding fputc('\n', stdin); after the while loop and it solves all three problems, but only when the input exceeds the character limit. If I write a sentence less than 100 chars, I have to hit enter twice for the program to continue.
  4. setbuf(stdin, NULL) is one of the solutions given, but one of the comments says "mucking around with setbuf is even worse [than fflush]"
Ken White
  • 123,280
  • 14
  • 225
  • 444
Frostbiyt
  • 23
  • 5
  • 1
    Stripping the newline is a bit of a nuisance. See [Removing trailing newline character from fgets() input](https://stackoverflow.com/questions/2693776) for canonical techniques. – Steve Summit Feb 15 '23 at 02:12
  • 2
    In general, you don't want `fgets` to overflow. The buffer you give it should be comfortably larger than the largest input you expect. Overflow should be a truly exceptional event. But you have to decide how you want to deal with it: (a) ignore the possibility, (b) discard too-long input, (c) process the too-long input. `fgets` assumes you might want to do (c), which is why it leaves the too-long input on the buffer. – Steve Summit Feb 15 '23 at 02:13
  • 2
    So if you want to do (b), you do need an input-flushing loop, but *only if `fgets` didn't read a whole line,* i.e. only if the returned buffer *didn't* have a newline. – Steve Summit Feb 15 '23 at 02:15
  • If fgets doesn't have a \n at the end then you didn't get the whole string. If it does have a \n at the end then you did and you can remove it. `fflush(stdin)` is undefined behaviour https://www.geeksforgeeks.org/use-fflushstdin-c/ – Jerry Jeremiah Feb 15 '23 at 02:15
  • There are a number of Q's here on SO detailing various tricks for handling this situation, efficiently, in all its varieties. But if it's all just too much bother, you might be able to use [`getline`](https://linux.die.net/man/3/getline) instead. – Steve Summit Feb 15 '23 at 02:18
  • 2
    And that explains why `fgets()` doesn't automatically strip the newline for you. Its presence conveys important information: that an input line has been consumed to its end. – John Bollinger Feb 15 '23 at 02:22
  • Thanks for the info @SteveSummit. I have to use c99 for my assignment, so `getline` won't work unfortunately, is there anything similar that I could use? If not, here's the solution I came up with based on the information you provided, is there a more elegant way to accomplish this? `fgets(input->sentence, 100, stdin);` `int temp = strcspn(target, "\n");` `if(temp == chars - 1) while ((getchar()) != '\n');` `else target[temp] = '\0';` – Frostbiyt Feb 15 '23 at 03:27
  • 1
    @Frostbiyt: The line `while ((getchar()) != '\n');` is not ideal, because it may create an infinite loop if `getchar()` for some reason returns `EOF`. That is why I also check for `EOF` in my answer to your question. The function `getchar` can return `EOF` if an error occurs on the input stream or end-of-file is reached. End-of-file can be reached for example if the input is being piped from a file or if the user presses a special key combination (which depends on which operating system is being used) to signal end-of-file. – Andreas Wenzel Feb 15 '23 at 03:44
  • @Frostbiyt: In the code you posted in your previous comment, you are telling `fgets` to write to `input->sentence`, but then you are using `strcspn` on `target`. This does not make sense, unless the expressions `input->sentence` and `target` both point to the same string. – Andreas Wenzel Feb 15 '23 at 04:05
  • I moved the code to a new function in my program, but wanted my comment to be consistent with my question, but I missed changing the second `target` to `input->sentence`, I'll edit. Or not, is there a time limit on editing a comment? – Frostbiyt Feb 15 '23 at 04:09
  • @Frostbiyt: Comments can only be edited 5 minutes after being posted. This time limit only applies to comments. – Andreas Wenzel Feb 15 '23 at 04:16
  • 1
    Frostbiyt, given input might be larger than desired, what do you want to happen to that extra input and do you want your input function to alert you that extra existed and tossed? – chux - Reinstate Monica Feb 15 '23 at 04:19

2 Answers2

2

I recommend that you create your own function that calls fgets, verifies that the entire line was read in and removes the newline character. If the function fails due to the input being too long, you can print an error message, discard the remainder of the line and reprompt the user. Here is a function that I have written myself for this purpose some time ago:

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

//This function will read exactly one line of input from the
//user. It will remove the newline character, if it exists. If
//the line is too long to fit in the buffer, then the function
//will automatically reprompt the user for input. On failure,
//the function will never return, but will print an error
//message and call "exit" instead.
void get_line_from_user( const char prompt[], char buffer[], int buffer_size )
{
    for (;;) //infinite loop, equivalent to while(1)
    {
        char *p;

        //prompt user for input
        fputs( prompt, stdout );

        //attempt to read one line of input
        if ( fgets( buffer, buffer_size, stdin ) == NULL )
        {
            printf( "Error reading from input!\n" );
            exit( EXIT_FAILURE );
        }

        //attempt to find newline character
        p = strchr( buffer, '\n' );

        //make sure that entire line was read in (i.e. that
        //the buffer was not too small to store the entire line)
        if ( p == NULL )
        {
            int c;

            //a missing newline character is ok if the next
            //character is a newline character or if we have
            //reached end-of-file (for example if the input is
            //being piped from a file or if the user enters
            //end-of-file in the terminal itself)
            if ( !feof(stdin) && (c=getchar()) != '\n' )
            {
                printf( "Input was too long to fit in buffer!\n" );

                //discard remainder of line
                do
                {
                    if ( c == EOF )
                    {
                        printf( "Error reading from input!\n" );
                        exit( EXIT_FAILURE );
                    }

                    c = getchar();

                } while ( c != '\n' );

                //reprompt user for input by restarting loop
                continue;
            }
        }
        else
        {
            //remove newline character by overwriting it with
            //null character
            *p = '\0';
        }

        //input was ok, so break out of loop
        break;
    }
}

Here is a demonstration program in which I apply my function to your code:

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

//forward declaration
void get_line_from_user( const char prompt[], char buffer[], int buffer_size );

int main( void )
{
    char sentence[100];
    char messageID[20];

    get_line_from_user( "Enter sentence: ", sentence, sizeof sentence );
    get_line_from_user( "Enter message ID: ", messageID, sizeof messageID );

    printf(
        "\n"
        "The following input has been successfully read:\n"
        "sentence: %s\n"
        "message ID: %s\n",
        sentence, messageID
    );
}

//This function will read exactly one line of input from the
//user. It will remove the newline character, if it exists. If
//the line is too long to fit in the buffer, then the function
//will automatically reprompt the user for input. On failure,
//the function will never return, but will print an error
//message and call "exit" instead.
void get_line_from_user( const char prompt[], char buffer[], int buffer_size )
{
    for (;;) //infinite loop, equivalent to while(1)
    {
        char *p;

        //prompt user for input
        fputs( prompt, stdout );

        //attempt to read one line of input
        if ( fgets( buffer, buffer_size, stdin ) == NULL )
        {
            printf( "Error reading from input!\n" );
            exit( EXIT_FAILURE );
        }

        //attempt to find newline character
        p = strchr( buffer, '\n' );

        //make sure that entire line was read in (i.e. that
        //the buffer was not too small to store the entire line)
        if ( p == NULL )
        {
            int c;

            //a missing newline character is ok if the next
            //character is a newline character or if we have
            //reached end-of-file (for example if the input is
            //being piped from a file or if the user enters
            //end-of-file in the terminal itself)
            if ( !feof(stdin) && (c=getchar()) != '\n' )
            {
                printf( "Input was too long to fit in buffer!\n" );

                //discard remainder of line
                do
                {
                    if ( c == EOF )
                    {
                        printf( "Error reading from input!\n" );
                        exit( EXIT_FAILURE );
                    }

                    c = getchar();

                } while ( c != '\n' );

                //reprompt user for input by restarting loop
                continue;
            }
        }
        else
        {
            //remove newline character by overwriting it with
            //null character
            *p = '\0';
        }

        //input was ok, so break out of loop
        break;
    }
}

This program has the following behavior:

Enter sentence: This is a test sentence.
Enter message ID: This is another test sentence that is longer than 20 characters and therefore too long.
Input was too long to fit in buffer!
Enter message ID: This is shorter.

The following input has been successfully read:
sentence: This is a test sentence.
message ID: This is shorter.
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
  • Your `get_line_from_user()` is definitely one of the better `my_getline()` routines. Not a fan of `int size` vs `size_t` and all the printing and `exit()` in the code, yet still a good overall function flow. UV – chux - Reinstate Monica Feb 15 '23 at 04:25
  • @chux: The reason my function uses `int` instead of `size_t` is because that is also what `fgets` uses. If I used `size_t` instead, then I could not simply forward the function argument to `fgets`. – Andreas Wenzel Feb 15 '23 at 04:29
  • @chux: What is wrong with "all the printing" and my call to `exit`? Would you consider it better if I used the return value of the function to indicate an error? Or are you referring to something else? – Andreas Wenzel Feb 15 '23 at 04:41
  • Yes, better to indicate an error. Helper functions should not end code. – chux - Reinstate Monica Feb 15 '23 at 06:11
  • Many `fgets()` use a constant passed in at the size - and many of those are `size_t` that fit in the `int` range. `fgets()` using an `int` size if a design weakness - we do not have to continue it. Yet either way, that is not the main issue. Your handling of input data here is the good part. – chux - Reinstate Monica Feb 15 '23 at 06:15
1

Here's what I'm using to solve my issue, thanks to Steve Summit and Andreas Wenzel for their comments.

int getSentence(SearchResults* input){
    printf("Enter sentence:");
    fgets(input->sentence, 100, stdin);
    int temp = strcspn(input->sentence, "\n");
    if(temp < 100 - 1) input->sentence[temp] = '\0';
    else while ((temp = getchar()) != '\n' && temp != EOF);

    printf("Enter message ID:");
    fgets(input->messageID, 20, stdin);
    temp = strcspn(input->messageID, "\n");
    if(temp < 20 - 1) input->messageID[temp] = '\0';
    else while ((temp = getchar()) != '\n' && temp != EOF);

    return 1;
}

EDIT: For anyone else who has the same issue as me, see comments below for additional problems you may need to account for if you use this solution, such as fgets returning NULL.

Frostbiyt
  • 23
  • 5
  • 1
    Without seeing the definition of `.sentence` and `.messageID`, this solution looks questionable. – chux - Reinstate Monica Feb 15 '23 at 04:28
  • @chux my actual implementation is a bit different. I have defined the sentence and ID lengths in a header file and use that in place of the 20 and 100 both in this function an in the struct definition, so the size will always match, if that's your concern. – Frostbiyt Feb 15 '23 at 04:37
  • 2
    It is generally a good idea to check the return value of `fgets` before using the result of `fgets`. For example, if the user signals end-of-file on an empty line by pressing `CTRL-D` on Linux or `CTRL-Z` on Windows, then `fgets` will return `NULL` and will not write a null-terminated string to the memory buffer. However, your function call to `strcspn` requires that this memory buffer is null-terminated. This may cause your program to crash. But this possible issue is probably not relevant for your use-case. – Andreas Wenzel Feb 15 '23 at 05:04
  • @Frostbiyt There are weaknesses to this approach. Better as a helper function as suggested by Andreas Wenzel – chux - Reinstate Monica Feb 15 '23 at 05:09