[PATCH 1/2] Check return value of fclose()

[Alan Jenkins]

man close(2)
...
Not checking the return value of close() is a common but nevertheless
serious programming error. It is quite possible that errors on a
previous 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.

---
.../src/unix/filesystem/ZLUnixFileOutputStream.cpp | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp b/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
index 7370dfb..99b6a15 100644
--- a/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
+++ b/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
@@ -61,7 +61,8 @@ void ZLUnixFileOutputStream::write(const std::string &str) {

void ZLUnixFileOutputStream::close() {
if (myFile != 0) {
- ::fclose(myFile);
+ if (::fclose(myFile))
+ myHasErrors = true;
myFile = 0;
if (!myHasErrors) {
rename(myTemporaryName.c_str(), myName.c_str());
--
1.6.3.3

[Alan Jenkins]

SQLLite files are already updated using fsync(). This change
will make ZLUnixFileOutputStream safer in case of power failure
on certain filesystems.

More specifically, this change make it safer for me to run
FBReader on my e-book reader using UBIFS. Without this, there
is a risk that config files will be truncated to 0 bytes if the
battery runs out soon after changing a config option.

See <http://www.linux-mtd.infradead.org/faq/ubifs.html#L_atomic_change>

"Changing a file atomically means changing its contents in a way that
unclean reboots could not lead to any corruption or inconsistency in
the file. The only reliable way to do this in UBIFS (and in most of
other file-systems, e.g. JFFS2 or ext3) is the following:

* make a copy of the file;
* change the copy;
* synchronize the copy (see here);
* re-name the copy to the file (using the rename() libc function
or the mv utility).

Note, if a power-cut happens during the re-naming, the original file
will be intact because the re-name operation is atomic. This is a POSIX
requirement and UBIFS satisfies it.

Often applications do not do the third step - synchronizing the copy.
Although this is generally an application bug, the ext4 file-system has
a hack which makes sure the data of the copy hits the disk before the
re-name meta-data, which "fixes" buggy applications. However, UBIFS does
not have this feature [...]"

---
.../src/unix/filesystem/ZLUnixFileOutputStream.cpp | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp b/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
index 99b6a15..40fa692 100644
--- a/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
+++ b/zlibrary/core/src/unix/filesystem/ZLUnixFileOutputStream.cpp
@@ -61,6 +61,8 @@ void ZLUnixFileOutputStream::write(const std::string &str) {

void ZLUnixFileOutputStream::close() {
if (myFile != 0) {

+ if (::fsync(fileno(myFile)))
+ myHasErrors = true;
if (::fclose(myFile))

myHasErrors = true;
myFile = 0;

--
1.6.3.3