tags:

views:

431

answers:

11

Hi All,

Is it required to check the return value of fclose? If we have successfully opened a file, what are the chances that it may fail to close?

Thanks!

Regards, Jay

+12  A: 

When you fwrite to a file, it may not actually write anything, it may stay in a buffer (inside the FILE object). Calling fflush would actually write it to disk. That operation may fail, for example if you just ran out of disk space, or there is some other I/O error.

fclose flushes the buffers implicitly too, so it may fail for the same reasons.

Nicolás
+1  A: 

The fclose man page cites that it may fail for any of the reasons that close or fflush may fail.

Quoting:

The close() system call will fail if:

 [EBADF]            fildes is not a valid, active file descriptor.

 [EINTR]            Its execution was interrupted by a signal.

 [EIO]              A previously-uncommitted write(2) encountered an
                    input/output error.

fflush can fail for reasons that write() would fail, basically in the event that you can't actually write to/save the file.

rascher
+1  A: 

If, in the context of your application, you can think of something useful to do if fclose() fails, then test the return value. If you can't, don't.

anon
OK but you didn't give an example where you need to check!
AraK
Don't see that the question was asking for one.
anon
Doesn't he say: "what are the chances that it may fail to close?"
AraK
That suggests he want a percentage - I wouldn't know how to come up with one.
anon
+1  A: 

You must ALWAYS check result of fclose()

vitaly.v.ch
+1  A: 

One reason fclose can fail is if there is any data still buffered and the implicit fflush fails. What I recommend is to always call fflush and do any error handling there.

R Samuel Klatchko
What does that error handling consist of?
anon
@Neil - the same error handling the app would do if an write operation failed (fail the request, log an error, etc.).
R Samuel Klatchko
+1  A: 

I've seen many times fclose() returning non-zero.

And on careful examination found out that the actual problem was with the write and not fclose.

Since the stuff being written are being buffered before actual write happens and when an fclose() is called all the buffer is flushed. So any problem in writing of the buffered suff, say like disk full, appears during fclose(). As David Yell says, to write a bullet proof application you need to consider the return value of fclose().

codaddict
to add: The function returns zero on success, or EOF on failure
0A0D
+1  A: 

You could (and should) report the error, but in a sense, the stream is still closed:

After the call to fclose(), any use of stream results in undefined behavior.

LnxPrgr3
+1  A: 
  1. fclose() will flush any unwritten output (via fflush()) before returning, so the error results from the underlying write() won't be reported at fwrite() or fprintf() time, but when you do the fclose(). As a result, any error that write() or fflush() can generate can be generated by fclose().

  2. fclose() will also call close() which can generate errors on NFS clients, where the changed file isn't actually uploaded to the remote server until close() time. If the NFS server crashed, then the close() will fail, and thus fclose() will fail as well. This might be true of other networked filesystems.

geocar
+5  A: 

From comp.lang.c:

The fclose() call can fail, and should be error-checked just as assiduously as all the other file operations. Sounds pedantic, right? Wrong. In a former life, my company's product managed to destroy a customer's data by omitting a check for failure when closing a file. The sequence went something like (paraphrased):

stream = fopen(tempfile, "w"); 
if (stream == NULL) ... 
    while (more_to_write) 
        if (fwrite(buffer, 1, buflen, stream) != buflen) ... 
fclose (stream); 


/* The new version has been written successfully.  Delete 
 * the old one and rename. 
 */ 
remove (realfile); 
rename (tempfile, realfile);

Of course, what happened was that fclose() ran out of disk space trying to write the last couple blocks of data, so the `tempfile' was truncated and unusable. And since the fclose() failure wasn't detected, the program went right ahead and destroyed the best extant version of the data in favor of the damaged version. And, as Murphy would have it, the victim in this particular incident was the person in charge of the customer's department, the person with authority to buy more of our product or replace it with a competitor's product -- and, natch, a person who was already unhappy with us for other reasons.

It'd be a stretch to ascribe all of the ensuing misery to this single omission, but it may be worth pointing out that both the customer and my former company have since vanished from the corporate ecology.

CHECK THOSE FAILURE CODES!

0A0D
Haha. I too thought of that post when I read the question. I decided to not search for it though, and wrote about it from memory.
Alok
@Alok: Great post :).. eight years old but it still rings true
0A0D
A: 

Let's say you're generating data. You have old data that you fread() from a file, and then do some processing on the data, generate more data, and then write it to a new file. You are careful to not overwrite the old file because you know that trying to create new file might fail, and you would like to keep your old data in that case (some data is better than no data). After finishing all the fwrite()s, which all succeed (because you meticulously checked the return value from fwrite()), you fclose() the file. Then, you rename() your just-written file and overwrite the old file.

If fclose() failed because of write error (disk full?), you just overwrote your last good file with something that might be junk. Oops.

So, if it is critical, you should check the return value of fclose().

In terms of code:

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

int main(void)
{
    FILE *ifp = fopen("in.dat", "rb");
    FILE *ofp = fopen("out.dat", "wb");
    char buf[BUFSIZ];
    size_t n;
    int success = 1;

    if (ifp == NULL) {
        fprintf(stderr, "error opening in.dat\n");
        perror("in.dat");
        return EXIT_FAILURE;
    }

    if (ofp == NULL) {
        fclose(ifp);
        fprintf(stderr, "error opening out.dat\n");
        perror("out.dat");
        return EXIT_FAILURE;
    }

    while ((n = fread(buf, 1, sizeof buf, ifp)) > 0) {
        size_t nw;
        if ((nw = fwrite(buf, 1, n, ofp)) != n) {
            fprintf(stderr, "error writing, wrote %lu bytes instead of %lu\n",
                            (unsigned long)n,
                            (unsigned long)nw);
            fclose(ifp);
            fclose(ofp);
            return EXIT_FAILURE;
        }
    }
    if (ferror(ifp)) {
        fprintf(stderr, "ferror on ifp\n");
        fclose(ofp);
        fclose(ifp);
        return EXIT_FAILURE;
    }

#ifdef MAYLOSE_DATA
    fclose(ofp);
    fclose(ifp);
    rename("out.dat", "in.dat"); /* Oops, may lose data */
#else
    if (fclose(ofp) == EOF) {
        perror("out.dat");
        success = 0;
    }
    if (fclose(ifp) == EOF) {
        perror("in.dat");
        success = 0;
    }
    if (success) {
        rename("out.dat", "in.dat"); /* Good */
    }
#endif
    return EXIT_SUCCESS;
}

In the above code, we have been careful about fopen(), fwrite(), and fread(), but even then, not checking fclose() may result in data loss (when compiled with MAYLOSE_DATA defined).

Alok
it would be great if you gave an example of how to check the return value for completeness
0A0D
Done. I've made it a compile-time option to run the right code.
Alok
Great! Except you forgot to check a couple more statements :)
0A0D
A: 

In a sense, closing a file never fails: errors are returned if a pending write operation failed, but the stream will be closed.

To avoid problems and ensure (as far as you can from a C program), I suggest you:

  1. Properly handle errors returned by fwrite().
  2. Call fflush() before closing the stream. Do remember to check for errors returned by fflush().
Danilo Piazzalunga