fgets returns null on non empty file

Question

I am trying to read from a file a non specific number of integers in pairs. I also want to skip lines that start with #. My problem is that nothing gets printed. When I tried to printf the value returned by fgets, it printed null. I would really appreciate a little help since I am not very experienced with C and I would be very grateful if you did not focus on feof since I have already read why is feof bad.

The file looks like this:

#This must
#be
#skipped
1233 14432
4943928  944949
11233   345432

And the code is:

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

typedef struct{
int start;
int end;
}path;
int main()
{
    path* array;
    array=malloc(5*sizeof(path));
    if(array==NULL){
    printf("Error allocating memory\n");
    abort();
    }


    FILE* fd=fopen("File.txt","r");
    if(fd==NULL){
    printf("Error opening file\n");
    abort();
    }
    char buff[200];
    int counter=0;
    if(fopen==NULL){
       printf("Error opening file\n");
        abort();
    }
    char c;
    while(!feof(fd)||counter==6){
        fgets(buff,200,fd);
        c=buff[0];
        if(strcmp(buff[0],"#")){
            continue;
        }
        sscanf(&buff,"%d %d",array[counter].start,array[counter].end);
        printf("%d\t%d\n",array[counter].start,array[counter].end);
        counter++;
    }


    fclose(fd);
    free(array);
    return 0;
}

This statement if(fopen==NULL){ or this one if(strcmp(buff[0],"#")){ does not make sense. — Vlad from Moscow, Jan 09 '20 at 21:39
Yeah, don't `strcmp` to compare two individual `char`s. You can compare them with `==` — Christian Gibbons, Jan 09 '20 at 21:40
[Why `while(!feof(file))` is always wrong](https://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) — Barmar, Jan 09 '20 at 21:40
Exactly. You claim "fgets() is returning null", but you never look at the return value of fgets, so how would you know? It could be returning anything. — Lee Daniel Crocker, Jan 09 '20 at 21:57
"fgets returns null on non empty file" is not demonstrated by this code - return value never checked. — chux - Reinstate Monica, Jan 09 '20 at 22:04
Azzarian "I would be very grateful if you did not focus on feof since I have already read why is feof bad" which counter-productive as `while(!feof(fd) ...` is a _significant_ part of the problem. Consider fixing that and review compiler warnings. — chux - Reinstate Monica, Jan 09 '20 at 22:12
Does this answer your question? [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — Toby Speight, Jan 10 '20 at 15:07

score 1 · Answer 1 · answered Jan 09 '20 at 21:42

1

You should not check feof() in the while condition. See Why is “while ( !feof (file) )” always wrong?.

The loop should be:

while (fcounter < 5 && fgets(buff, 200, fd))

answered Jan 09 '20 at 21:42

Barmar

741,623
53
500
612

Luis Colorado · Accepted Answer · 2020-01-10T09:51:11.847

First, answering the title of your question: fgets() returns NULL at end of file and not when a file is empty.

Anyway, your test in the while loop is incorrect:

feof() only gives a true result when you have already tried read and you have already hit the end of file with an unsuccessful read. As read tries to give you as many bytes as it can... or none at all if end of file, the only way to get end of file condition is after you failed to read something. It is far better to check for fgets() result, as it returns NULL on being unable to read anything now. (and not in the last read) so
```
while(fgets(buff, sizeof buff, fd) != NULL)
```
or just
```
while(fgets(buff, sizeof buff, fd))
```
would be far better. Also, see how I use the sizeof operator to use the size of the used buffer, instead of repeating (and being error prone) the actual number of bytes at two places. If you decide to change the size of the buffer, you'll need also to change the actual number of bytes to read in the fgets() call, making the possibility of forgetting one of them an opportunity to run into trouble.
you order to stay in the loop only while !feof() OR when counter == 6 (first, this will make the control to enter the loop when counter is equal to 6, despite you have reached EOF or not, this cannot be correct) Think that you only get out of the loop when both conditions are false (this means feof() returns true and also counter != 6), you had better to write:
```
while(fgets(buff, sizeof buff, fd) && counter < max_number_of_iterations)
```
The test
```
if(strcmp(buff[0],"#"))
```
is also incorrect, as buff[0] is a character (indeed, it is the first character read in the buffer, and "#" is a string literal (not a character) Probably you got at least a warning from the compiler, from which you say no word. You had better to test both characters for equality, as in
```
if (buff[0] == '#')  /* this time '#' is a character literal, not a string literal */
```
in the line
```
if (fopen == NULL)
```
fopen by itself is a pointer to the library function fopen(3) which is not what you want (fopen is always != NULL) but
```
if (fd == NULL){
```
(which you do before, so you had better to eliminate al this code)
you define a char c;, then initialize it to the first char of buff, and then you don't use it at all. This has no impact in your code but it's bad style and confounds maintainers in the future.
in the line sscanf(&buff, "%d %d", .... you don't need to pass &buff, while buff is already a char pointer. It is better to pass it buff.n But instead, you need to pass pointers to the variables you are reading so you need it corrected into:
```
sscanf(buff, "%d%d", &array[counter].start, &array[counter].end);
```
not doing so will make an Undefined Behaviour that will be difficult to pursue, as the use of uninitialised variables (and more on, pointers to variables) will make the code probably work at first, but fail when it has got to production for a while... this is a very serious error.

Your code, with all these errors corrected, should look like:

pru.c

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

#define N (5)  /* I have  defined this constant so you can 
                * change its value without having to go all 
                * the code for occurrences of it and 
                * changing those */

typedef struct{
    int start;
    int end;
} path;

int main()
{
    path* array = malloc(N*sizeof(path)); /* better declare and init */
    if(array==NULL){
        printf("Error allocating memory\n");
        abort();  /* have you tried exit(EXIT_FAILURE); ?? */
    }

    FILE* fd=fopen("File.txt","r");
    if(fd==NULL){
        printf("Error opening file\n");
        abort();
    }
    char buff[200];
    int counter=0;
    while(fgets(buff, sizeof buff, fd) && counter < N){
        if(buff[0] == '#'){
            continue;
        }
        sscanf(buff, "%d %d", &array[counter].start, &array[counter].end);
        printf("%d\t%d\n", array[counter].start, array[counter].end);
        counter++;
    }

    fclose(fd);
    free(array);

    return 0;
}

Running the code shows:

$ pru
1233    14432
4943928 944949
11233   345432

with the File.txt you posted.

Finally, a hint:

Despite of your interest in knowing only the reasons of your loop falling, and not why feof() is of no use here (and many other things you just don't ask for and that are wrong in your code), if that is actually the case, you had better to post an example that only shows the failing behaviour as recommended by the page How to create a Minimal, Complete, and Verifiable example you should have read and which I recommend you to do.

fgets returns null on non empty file

2 Answers2

pru.c

Finally, a hint: