fgets where the number of char read is bigger than the size of the storage

Question

I'm looking at a code to learn things and I saw something like this :

void funRead(){
     char str[80];
     FILE *fi;
     fi = fopen("file","r");
     while(!feof(fi)){
        fgets(str, 200, fi);
        // do things
     }
     fclose(fi);
}

And it works! (It's quite a big code) I didn't understand why it was working so I tried to reproduce it (just this part, pretty much just the code above) and my program crashes (I'm doing it on Eclipse). It only works when I write

fgets(str, 80, fi);

Or another number < 80, otherwise it won't work.

Did I miss something?

EDIT : Screen capture of the part of the program I'm talking about https://gyazo.com/c8847ccc36bbbe7a406a3260db8dd358 Lines 4 & 26

You missed the `feof`, [which is always wrong](https://stackoverflow.com/questions/5431941). But you are correct that the second argument to `fgets` needs to be less than or equal to the buffer size. — user3386109, Apr 18 '18 at 01:31
It works _only_ because the _actual_ length of the lines read is 79 [or less]. The `200` is the maximum (wrong), but if all lines are 79, you squeak by. (e.g.) If you had a line that is 90, the program will have undefined behavior [and will probably segfault]. You'd overwrite the return address of the function and may destroy the value of `fi` while reading. — Craig Estey, Apr 18 '18 at 01:43
Please don't post pictures of code, copy & paste code in your question. — Pablo, Apr 18 '18 at 01:46
Undefined behaviour is undefined behaviour. The fact that the other code "works" doesn't mean anything, it might run without crashing but it doesn't mean that it is running correctly. You just got lucky that you didn't see a segfault right away. — Pablo, Apr 18 '18 at 01:48

score 3 · Accepted Answer · answered Apr 18 '18 at 01:36

fgets(str, 200, fi); is indeed wrong, because it is telling fgets to read up to 199 characters when in reality str can store up to 79 characters (not including the '\0'-terminating byte), that's why it "works"¹ with fgets(str, 80, fi);.

In general it's a good idea to use sizeof like this²:

char str[80];
fgets(str, sizeof str, fi);

With sizeof you will get the correct size, even if you later decide to change the size of str.

One thing that is really wrong is:

while(!feof(fi))
{
    ...
}

What the code should do is:

while(fgets(str, sizeof str, fi))
{
    // do things
}

that's the proper way to read the whole file.

Fotenotes

¹I put works in quotes because the way the code checks when to stop reading is incorrect. However fgets(str, 80, fi) is the correct call.

²Note that sizeof will give the correct size only if str is an array. If it is a pointer and you allocated memory for it, then you cannot use sizeof, for example:

size_t len = 80;
char *str = malloc(len);
if(str == NULL)
{
    // error handling
}

fgets(str, len, fi); // <-- do not use sizeof here, but the variable

First, thanks for all those explanations. Actually I was trying to figure out why the 'wrong' code I wrote was working, because yes I understand how the code **should** work — Daycopo, Apr 18 '18 at 01:50
@PaulMonties classic undefined behaviour. You just got lucky that it didn't crash right away. But it could have. — Pablo, Apr 18 '18 at 01:52

jat · Answer 2 · 2018-04-18T01:48:13.163

In your code you are defining a character array with size of 80 characters

char str[80];

fgets(str, 200, fi);

Above statement means that it is reading 200 characters from file "fi" in the character array "str" of size 80. You cannot copy 200 characters in an array of size 80.

Either make character array bigger or read 80 or less characters at a time from the file.

EDIT: The reason program crashes is because it is accessing illegal memory locations. This does not mean it will necessary crash if you index a location which is greater than size of array, It just means it can crash if it is not allowed to access that memory location. So that is the reason, the code you mentioned is probably working(purely on chance), although that 200 part is wrong. If it ran then run it multiple times it will probably crash atleast once.

Yes I know that and I understand it, but I have a code under my eyes where there is pretty much the code I wrote and it works, that's what I dont understand. Is there any way? — Daycopo, Apr 18 '18 at 01:36

fgets where the number of char read is bigger than the size of the storage

2 Answers2