Re: [PATCH] bundle, fast-import: detect write failure

From: Jim Meyering <jim@meyering.net>
Date: Jan 10, 2008, 12:26 UTC
Message-ID: <87myrdhnn5.fsf@rho.meyering.net>
In-Reply-To: <alpine.LSU.1.00.0801101204120.31053@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 5 quoted lines

> However, you also have this:
>
>> -	close(keep_fd);
>> +	if (close(keep_fd))
>> +		die("failed to write keep file");

Yes.  I mentioned that in the commit log:

    * bundle.c (create_bundle): Die upon write failure.
    * fast-import.c (keep_pack): Die upon write or close failure.

But even the summary is accurate if you interpret "write" not as the syscall, but as the semantic push-data-through-OS-to-disk operation.

> I recently read an article which got me thinking about close().  The
> author maintained that many mistakes are done by being overzealously
> defensive; die()ing in case of a close() failure (when open() succeeded!)
> might be just wrong.

No. Whether open succeeded is a separate matter. Avoiding an unreported write (or close-writable-fd) failure is not being "overzealously defensive."

>From "man 2 close",

    -------------
    NOTES
       Not  checking  the return value of close() is a common but nevertheless
       serious programming error.  It is quite possible that errors on a  pre-
       vious  write(2) operation are first reported at the final close().  Not
       checking the return value when closing the file may lead to silent loss
       of data.  This can especially be observed with NFS and with disk quota.
    -------------