Reading files with fgets()-CodePudding

I have a doubt about reading files in C using fgets(). I've seen people use loops in order to do this, but I skip the loop part, doing this instead.

What's the difference between using a loop and my way?

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

int main() {
    FILE *file = NULL;
    char string[30];
    file = fopen("test.txt", "r"); //test.txt contains "Hello world!"

    if (file == NULL) {
        puts("ERROR");
        return 1;
    }

    fgets(string, 30, file);

    puts(string);

    fclose(file);

    return 0;

}

Outputs: Hello world!

CodePudding user response：

What's the difference between using a loop and my way?

OP's way has many problems.

Wrong buffer size

fgets(string, 30, file); overstates the buffer size of 10 allowing undefined behavior (UB) due to a potential buffer overflow.

Input result not checked

fgets(string, 30, file); does not check the return value of fgets().
Until the return value is checked, the contents of string are not known to be updated correctly.

Extra '\n'

puts(string); appends an extra '\n'.

The entire file is not certainly read

A single read might not read the entire contents. Use a loop.

Alternative: read until fgets() returns NULL.

while (fgets(string, sizeof string, file)) {
  fputs(string, stdout);
}

CodePudding user response：

From the man page:

The fgets() function shall read bytes from stream into the array pointed to by s until n-1 bytes are read, or a is read and transferred to s, or an end-of-file condition is encountered. A null byte shall be written immediately after the last bytes read into the array.

According to this, fgets stops reading when it encounters a newline, an EOF condition, an input error, or when it has read n - 1 characters. So your approach only reads one line from the file. That's well and good if you need to read only a single line.

To read a whole file line by line, fgets is called in a loop until an EOF condition is reached. Another way would be to read the whole file into a buffer with fread, and then parse it. Or read it character by character by calling getc in a loop.

EDIT: In your code, fgets is trying to read (n - 1) 29 bytes of memory whereas you allocated only 10 bytes for the buffer. This leads to undefined behaviour. The memory not allocated should not be read. Use sizeof (string) instead.

"Hello World!" can not fit in a buffer you allocated 10 bytes for.

RETURN VALUE:

Upon successful completion, fgets() shall return s. If the stream is at end-of-file, the end-of-file indicator for the stream shall be set and fgets() shall return a null pointer. If a read error occurs, the error indicator for the stream shall be set, fgets() shall return a null pointer, and shall set errno to indicate the error.

You didn't check the return value of fgets.