Infinite while loop when reading file

Question

I am getting an infinite loop (in while loop) with this function. I am new to working with files, so I feel like I am missing something...I can't see what is wrong.

void cipher(FILE* password_ptr,int n)
{    
        if (password_ptr == NULL)
        {
                printf("Error:password_ptr points to null");
                return;
        }
        while(!feof(password_ptr))
        {
                fseek(password_ptr, 0, SEEK_CUR); // don't move
                int en=fgetc(password_ptr)+n;

                fseek(password_ptr, -1, SEEK_CUR); // move backwards one character
                if(fputc(en,password_ptr)!=en)
                {
                printf("Error:fputc didn't work");
                }
                fseek(password_ptr, 0, SEEK_CUR);
        }
        fclose(password_ptr);
};

Thanks!

your own code comments tell you why - apply 1st to the last fseek if you really cannot see it — Aaron Gage, Mar 04 '12 at 04:02

score 4 · Accepted Answer · edited May 23 '17 at 12:21

A side effect of calling fseek() is that the EOF indication on the file gets cleared:

C99 7.19.9.2/5 The fseek function:

After determining the new position, a successful call to the fseek function undoes any effects of the ungetc function on the stream, clears the end-of-ﬁle indicator for the stream, and then establishes the new position.

Note that your code also uses the common anti-pattern of a loop controlled by the feof() function. That function will not return EOF until after an I/O operation gets you to that point (sets the end-of-file indicator). In other words, even when you enter the loop, the fgetc() may fail due to being at the end of the file (that's what will set the end-of-file indicator). But then a subsequent seek will clear that indicator. In the meantime, you'll have operated on the EOF as if it were a normal, successful read.

See:

You might want to try the following loop instead:

int c;
while((c = fgetc(password_ptr)) != EOF)
{
    int en= c+n;

    fseek(password_ptr, -1, SEEK_CUR); // move backwards one character
    if(fputc(en,password_ptr)!=en)
    {
        printf("Error:fputc didn't work");
        break;
    }
}

You also need to think about how you want this bit of code to handle a situation where en is out of the range of an unsigned char. Since fputc() converts the character to be written to an unsigned char before writing it to the stream, if en is outside of that range the "fputc didn't work" error will be displayed. This might happen, for example, if adding n to the character read by fgetc() is larger than 255.

@spatara: Note: my original suggested fix had `c` declared as an `int`, which might result in `EOF` not being detected (another common bug with handling `EOF`) - `c` should be an `int` type to make sure `EOF` is properly detected. — Michael Burr, Mar 11 '12 at 23:20

Infinite while loop when reading file

1 Answers1