Why is my code ignoring my fgets() and giving it a 0 value?

Question

There is buffer overflow causing fgets to get a new line without taking in any stdin from the user. If I use a function to clear the buffer before stringswap() is used the program runs fine and I am able to change the value of the string and have it printed. However I want to know where the buffer overflow originates from to not have this happen again.

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

char arg[100];

void Options();
int AskChoice();
void stringswap();
void clear();

int main()
{
    Options();
    return 0;
}

void Options()
{
    int choice = 0;
    choice = AskChoice();
    if (choice == 1) {
        stringswap(arg);
        printf("Your word is %s\n", arg);
        Options();
    }
    else if (choice == 2) {
        printf("Goodbye");
    }
    else {
        Options();
    }
}

int AskChoice()
{
    int choice = 0;
    char Choice[2];
    printf("Enter 1 to say a word, enter 2 to exit program.\n");
    fgets(Choice, 2, stdin);
    choice = atoi(Choice);
    printf("Your choice is %d\n", choice);
    return choice;
}

void stringswap(char* input)
{
    printf("Enter a new string");
    fgets(input, 50, stdin);
}

void clear()
{
    while (getchar() != '\n')
        ;
}

I expect to type in a word and get it repeated back to me, but instead it's completely ignored and I get a blank space returned back.

Pressing `Enter` on a keyboard also adds `\n` at the end of the buffer, which iirc, is why you are getting a blank return. This is the case for scanf, but I don't remember if it's the same with fgets. https://stackoverflow.com/questions/31725517/remove-n-after-scanf-which-read-integer — mlizbeth, Feb 01 '19 at 01:04
*"There is a buffer overflow"* - there certainly is, and you seem to know it. So... fix it? `fgets` is writing *three* chars, including the terminator. Your program invokes undefined behavior. Fix that *first*. — WhozCraig, Feb 01 '19 at 01:06
Your input will require `"1\n\0"` (3 chars). When you specify `2` as the limit for `fgets`, `fgets` writes `"1\0"` to the buffer, leaving `'\n'` in `stdin` -- unread. Your next call to `fgets` reads up to the first `'\n'` (which is the 1st character in `stdin` giving the appearance that `fgets` is being skipped). It isn't, it's just reading the `'\n'` you left in `stdin`. — David C. Rankin, Feb 01 '19 at 01:15
In addition to what @DavidC.Rankin commented - A good way to ensure your `fgets()` calls do not *ever* over-run buffers, when using a character array as a buffer (as opposed to a pointer), is to pass `sizeof(target_buffer)` as the second argument. For example: `char buffer[500]; fgets(buffer, sizeof(buffer), stdin);`. It also frees you from having to update a size parameter if `buffer`'s size changes. This works for `AskChoice()`, but not `stringswap()` because the size of the buffer (`input` points to) in `stringswap()` is no longer known. — Kingsley, Feb 01 '19 at 01:26
While @Kingsley makes a good suggestion, note `sizeof (Choice)` will still be `2` in this case. Following the general rule ***Never Skimp on Buffer Size***, `#define MAXCHOICE 512 ... char Choice[MAXCHOICE];` and then following the advise above, and then validating that `size_len = strlen (Choice); if (len == MAXCHOICE - 1 && Choice[len - 1] != '\n') /* handle input exceeds buffer size error */` — David C. Rankin, Feb 01 '19 at 01:26
Thank you, it was the fgets(choice, 2, stdin) that was overflowing. I thought 2 was enough space to get specifically a 1 or 2 plus the null terminator but that seemed to be one less. — Remilia Scarlet, Feb 01 '19 at 01:55

score 1 · Answer 1 · answered Feb 01 '19 at 02:40

2 is enough to get 1 char and then append a null character. Thus only consuming 1 character from the user per call. No enough per OP's needs.

Use a larger buffer.

// char Choice[2];
char Choice[100];
printf("Enter 1 to say a word, enter 2 to exit program.\n");
// fgets(Choice, 2, stdin);
fgets(Choice, sizeof Choice, stdin);

Why is my code ignoring my fgets() and giving it a 0 value?

1 Answers1