[SOLVED] Gets() behaving like fgets()?


#1

I was testing out exercise 24 in Learn C the Hard Way and noticed that the only difference I was experiencing between fgets() and gets() was the syntax for calling it… fgets() seems to overflow into other variables just like gets(). Any ideas why that’s happening? Here’s what the program execution looks like with MAX_DATA set to 10:
Screenshot
(Note that the only things I entered were the first long number and the eye color)

Thanks in advance!


#2

From man 3 gets and man 3 fgets

gets()  reads  a line from stdin into the buffer pointed to by s until either a terminating newline or EOF, which it replaces with a null byte ('\0').  No check for buffer overrun is performed (see BUGS
       below).

fgets() reads in at most one less than size characters from stream and stores them into the buffer pointed to by s.  Reading stops after an EOF or a newline.  If a newline is read, it is stored into the
       buffer.  A terminating null byte ('\0') is stored after the last character in the buffer.

gets() returns s on success, and NULL on error or when end of file occurs while no characters have been read.  However, given the lack of buffer overrun checking, there can be  no  guarantees  that  the
       function will even return.

In the Bugs section:

Never use gets().  Because it is impossible to tell without knowing the data in advance how many characters gets() will read, and because gets() will continue to store characters past  the  end  of  the
       buffer, it is extremely dangerous to use.  It has been used to break computer security.  Use fgets() instead.

#3

Thanks, that makes sense to me. The problem is I am using fgets() in my code and it is behaving unsafely like gets().
Here’s a screenshot of what I think is a relevant portion of the code:


#4

And here’s another example of what breaking the program looks like:

It doesn’t even seem to exit on the error!


#5

Do you use valgrind? Does it have any output that could help?


#6

No… I’m not sure how to use Valgrind. By the way, I get similar behavior in exercise 25 with MAX_DATA set to 5. I can type in dddddddddd20 to fill all the fields:


#7

If valgrind is installed it’s a simple command for the basic use:

$ valgrind ./ex24

But I usually run with a couple extra switches:

$ valgrind --leak-check=full --track-origins=yes ./ex24

Also if you’ve made it to ex24 without using gdb, I’m impressed.


#8

Thanks so much, I’m installing Valgrind right now and then I’ll try that.


#9

I’ve used a little GDB, but I’m still learning. No Valgrind, though. It seems to be happy…

EDIT: I get the same response with your flags


#10

Worth a shot. Sometimes it catches some weird memory runoff’s and tracks em down. I’m a student too. But gdb and valgrind are both very valuable. gdb is especially awesome once you learn a few commands like explore, print, break, info, watch, make, run, start, step, next, continue, finish, set, delete, clear.

EDIT: I could still be forgetting some. But those all have been good to me in general gdb usage.


#11

Yeah, break is SUPER useful I’ve found. Here’s the output of Valgrind -> [|]

Thanks for the help so far!


#12

I’d say it would be well worth your time at this point to play around in gdb, use the help and apropos commands to get familiar with gdb commands. The more you dabble the more you see what I mean. That gdb is very powerful if you know your way around.


#13

Ok, thank you. I will do that.


#14

As for the issue with fgets, that’s beyond me lol. I’m not to that chapter yet. But I’m sure someone can shine some light on things.


#15

That’s super bizarre. The man page says that fgets will add the \0 terminator if the fgets does NOT have an error. It looks like one of three things:

  1. The you.first_name is not actually MAX_DATA in length.
  2. Your structs are not zeroed out so you have a good base. Honestly this shouldn’t matter since fgets is supposed to add the \0 but do that anyway and see if that solves the problem.
  3. This might be the situation where fgets is told to read MAX_DATA -1, it doesn’t read the \n by that byte count, so then it refuses to add the \0. I’d have to research more if that’s a thing but I vaguely remember that.

Try #2, as a quick fix. Remember you can easily zero out a struct by setting one attribute to 0 or NULL on initialization:

struct MyThing stuff = {.name = NULL};

You’ll have to forgive me if that syntax is off since I’m a little rusty on C these days. If none of that works and you’re using malloc to make the data structs then use calloc instead. Do NOT try to use memset to zero out the memory. That produces errors and you’ll get the size wrong all the time.


#16

Thanks! I tried initializing the strings with { 0 } but that didn’t seem to do anything… Initializing them with 0 and NULL didn’t work either. It may very well be the third thing you said. I noticed that the minimum number of characters needed to overflow is 8, not 10 (which is the value of MAX_DATA). Any ideas how I might fix that? I’ll try some things meanwhile. I’m also pretty sure it’s a problem with input, because it doesn’t actually seem to be overflowing in memory, and Valgrind is happy as a clam…

UPDATE: Telling fgets() that the size is MAX_DATA - 2 didn’t work :frowning:


#17

Check this out, maybe it’ll clarify the string initialization?

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

char **strings;

int main(int argc, char *argv[])
{
    strings = calloc(100, sizeof(char) * 100);
    strings[0] = calloc(100, sizeof(char));
    strings[0] = strncpy(strings[0], "Something\n", 100);

    strings[1] = calloc(100, sizeof(char));
    strings[1] = strncpy(strings[1], "Something else\n", 100);
    
    int i = 0;
    for (i = 0; i < 100; i++) {
        printf(strings[i]);
        free(strings[i]);
    }
    free(strings);
}

#18

I did some research and the problem doesn’t seem to be what I originally thought. fgets() is correctly reading in the string up to MAX_DATA and is correctly terminating it with '\0', but the excess characters I typed are being stored in some buffer and entered into the next fgets() calls.

I was able to solve it by checking whether the input contained "\n" and if so flushed STDIN. Here’s a snippet of the code:

// ...
void flush_stdin() {
  int ch;
  ch = fgetc(stdin);
  do { ch = fgetc(stdin); } while (ch != EOF && ch != '\n');
}

int main(int argc, char *argv[]) {
    Person you = { .age = 0 };
    int i = 0;
    char *in = NULL;

    printf("What's your first name? ");
    in = fgets(you.first_name, MAX_DATA - 1, stdin);
    check(in != NULL, "Failed to read first name");
    if (!strstr(you.first_name, "\n")) flush_stdin();

    printf("What's your last name? ");
    in = fgets(you.last_name, MAX_DATA - 1, stdin);
    check(in != NULL, "Failed to read last name");
    if (!strstr(you.last_name, "\n")) flush_stdin();

    printf("How old are you? ");
    int rc = fscanf(stdin, "%d", &you.age);
    check(rc > 0, "You have to enter a number");

    printf("What color are your eyes:\n");
    for (i = 0; i <= OTHER_EYES; i++) {
        printf("%d) %s\n", i + 1, EYE_COLOR_NAMES[i]);
    }
    printf("> ");

    int eyes = -1;
    rc = fscanf(stdin, "%d", &eyes);
    check(rc > 0, "You have to enter a number");

    you.eyes = eyes - 1;
    check(you.eyes <= OTHER_EYES && you.eyes >= 0, "Do it right, that's not an option");
// ...

#19

No, I don’t think that’s it either. There is something you’re doing that either doesn’t initialize the variables correctly, OR is pointing fgets at the wrong thing. Try this:

That’s my code for the problem. Take that, compile it and run it just like you’re doing here. If mine works and yours doesn’t then do this:

diff zeds_ex24.c my_ex24.c

That’ll tell you what’s wrong. Then, report back because I am dying out of curiosity.


#20

I have the same original problem with your code as well… so weird! This was the only thing that helped.