Reaching EOF with fgets

Question

I'm writing a function that perform some authentications actions. I have a file with all the user_id:password:flag couples structured like this:

Users.txt

user_123:a1b2:0 user_124:a2b1:1 user_125:a2b2:2

This is the code:

int main(){
    /*...*/

    /*user_id, password retrieving*/
    USRPSW* p = malloc(sizeof(USRPSW));
    if(p == NULL){
        fprintf(stderr, "Dynamic alloc error\n");
        exit(EXIT_FAILURE);
    }
    memset((void*)p, 0, sizeof(USRPSW));

    if(usr_psw_read(acc_sock_ds, p->user_id, USR_SIZE) <= 0){
        printf("Failed read: connection with %s aborted.\n",
                 inet_ntoa(client_addr.sin_addr));
        close(acc_sock_ds);
        continue;
    }

    if(usr_psw_read(acc_sock_ds, p->password, PSW_SIZE) <= 0){
        printf("Failed read: connection with %s aborted.\n",
                 inet_ntoa(client_addr.sin_addr));
        close(acc_sock_ds);
        continue;
    }

    /*Authentication through user_id, password*/
    FILE *fd;
    fd = fopen(USERSFILE, "r");
    if(fd == NULL){
        fprintf(stderr, "Users file opening error\n");
        exit(EXIT_FAILURE);
    }

    char *usr_psw_line = malloc(USR_SIZE+PSW_SIZE+3+1);
    if(usr_psw_line == NULL){
        fprintf(stderr, "Dynamic alloc error\n");
        exit(EXIT_FAILURE);
    }

    while(1){

        memset((void*)usr_psw_line, 0, sizeof(USR_SIZE+PSW_SIZE+3+1));
        fgets(usr_psw_line, USR_SIZE+PSW_SIZE+3+1, fd);
        printf("%s\n", usr_psw_line);
        fseek(fd, 1, SEEK_CUR);


        /*EOF management*/
        /*usr_id - password matching checking */

    }   
/*...*/    
}

How can I manage the EOF reaching? I saw that when EOF is reached fgets doesn't edits anymore the usr_psw_line but neither returns a NULL pointer. If EOF is reached it mean that no match are found in the users file and the loop breaks.

Can someone give me some tips or suggests?

Ikon

If the end of file is reached without reading any characters, fgets is obliged to return NULL. Anyway, you could check feof(fd) after fgets didn't read anything. – Mandolin

I'm afraid that EOF is not set. I only write a file with records inside.I should also explicitly set the EOF? How do you do this? – Ikon

Isn't it about reaching the end of file while reading? You don't set EOF, if the end of the file is reached, the next attempt to read from it will set the flag in *fd, so feof(fd) will then return true if everything works as it should. If everything is not working as it should, hmmm. – Mandolin

EOF is a condition, not part of the stream. Imagine the stream is water from a tap. When you open the tap until it exhausts do you get more water (in another color?) to signal there is no more water? – Saari

Answer 1

fgets() return a null pointer when it reaches end-of-file or an error condition.

(EOF is a macro that specifies the value returned by certain other functions in similar conditions; it's not just an abbreviation for the phrase "end of file".)

You're ignoring the result returned by fgets(). Don't do that.

Note that just checking feof(fd) won't do what you want. feof() returns a true result if you've reached the end of the file. If you encounter an error instead, feof() still returns false, and you've got yourself an infinite loop if you're using feof() to decide when you're done. And it doesn't return true until after you've failed to read input.

Most C input functions return some special value to indicate that there's nothing more to read. For fgets() it's NULL, for fgetc() it's EOF, and so forth. If you like, you can call feof() and/or ferror() afterwards to determine why there's nothing more to read.

Despinadespise

(said with a smile) Didn't I just say that in my earlier answer? – Niklaus

@SteveValliere More or less, but I tried to be more explicit about the underlying concepts. – Despinadespise

Answer 2

You might want to try something like this in your loop:

while(1)
{
    memset((void*)usr_psw_line, 0, sizeof(USR_SIZE+PSW_SIZE+3+1));
    if( !fgets(usr_psw_line, USR_SIZE+PSW_SIZE+3+1, fd)
     || ferror( fd ) || feof( fd ) )
    {
        break;
    }
    printf("%s\n", usr_psw_line);
    fseek(fd, 1, SEEK_CUR);

    /*EOF management*/
    /*usr_id - password matching checking */

}

With the extra code, the loop will terminate if fgets returns NULL (no more data to read) or if you're read the EOF mark or had any error on the file. I'm sure it is overkill, but those tests have always worked for me.

Niklaus

It would be best (IMO) to use while (fgets(usr_psw_line, sizeof(usr_psw_line), fd) != 0) for the loop condition. There's really not much need to zero the memory before the read. And there's no benefit to testing with ferror() or feof() there (since they'd only evaluate to true if the test on the return from fgets() returns NULL; therefore, they never cause the loop to exit). – Azide

Agreed. I was trying to leave as much original code intact as I could so that just the parts that directly answered the question were clear. But I do exactly what you describe in all of my file reading loops. – Niklaus

@JonathanLeffler "testing with ferror() ... only evaluate to true if the test on the return from fgets() returns NULL" --> I do not think so in a corner case. ferror() returns the status of the error flag which may have been set prior to the fgets() call. fgets() does not clear that flag so fgets() may return non-NULL yet ferror() returns true. – Euphonize

@chux: My argument is based on the proposition that if the error flag is set when fgets() is called, fgets() must fail. However, I can't actually find verbiage in the standard that mandates that, though I'd be astonished to find an implementation that didn't behave like that. If you track the state of stream, you won't be using it after a failure unless you use clearerr(). There is an implausible edge case available. Testing feof() is pointless. I stand by my observation for everyday use. If I was worried about the error state of the stream, I'd use ferror() on entry to the function. – Azide

@JonathanLeffler Agree about everyday use, feof(), obscureness and good code would test ferror(), yet it was the only that caught my eye. fgets() does not automatically return NULL if the stream's error indicator is set, but will return NULL is a read error occurs (which has the side effect of setting the error indicator). – Euphonize

The code "sizeof(USR_SIZE+PSW_SIZE+3+1)" in the example is always computing the size of an integer - on many modern machines it's always going to be 4, regardless of the values of USR_SIZE and PSW_SIZE -- "sizeof(42)" (or any other smallish number) would return the same value. I'm guessing the enclosing "sizeof(...)" is a mistake. Zeroing the entire buffer is overkill anyway - if one were suspicious of the behavior of fgets() - a C string based function - then "usr_psw_line[0] = '\0';" would be sufficient. – Pixie

And if feof() and/or ferror() are going to be used, they should probably be called either before the call to fgets() - in order to determine if the given file is already in an EOF or error state from prior activities - or after fgets() returns NULL, to determine why fgets() has returned NULL. In the current form, they do little other than to misdirect someone looking at the code. – Pixie