Commit 604e5c18 authored by tnagel's avatar tnagel Committed by Commit bot

Abort ImportantFileWriter::WriteFileAtomically() on Flush() error.

As discussed on chromium-os-dev [1], if there is no fsync() after writing to the
temp file, atomicity is not guaranteed.  Thus, to be able to guarantee
atomicity, we must check the return value of Flush() and bail in case Flush()
wasn't successful.

I'll update the UMA histogram enum in a separate CL.

[1] https://groups.google.com/a/chromium.org/forum/?hl=en#!topic/chromium-os-dev/Eef8gNIRwjc

BUG=none

Review URL: https://codereview.chromium.org/663463002

Cr-Commit-Position: refs/heads/master@{#300230}
parent cf93cf2f
......@@ -33,6 +33,7 @@ enum TempFileFailure {
FAILED_CLOSING,
FAILED_WRITING,
FAILED_RENAMING,
FAILED_FLUSHING,
TEMP_FILE_FAILURE_MAX
};
......@@ -69,7 +70,7 @@ bool ImportantFileWriter::WriteFileAtomically(const FilePath& path,
CHECK_LE(data.length(), static_cast<size_t>(kint32max));
int bytes_written = tmp_file.Write(0, data.data(),
static_cast<int>(data.length()));
tmp_file.Flush(); // Ignore return value.
bool flush_success = tmp_file.Flush();
tmp_file.Close();
if (bytes_written < static_cast<int>(data.length())) {
......@@ -79,6 +80,12 @@ bool ImportantFileWriter::WriteFileAtomically(const FilePath& path,
return false;
}
if (!flush_success) {
LogFailure(path, FAILED_FLUSHING, "error flushing");
base::DeleteFile(tmp_file_path, false);
return false;
}
if (!base::ReplaceFile(tmp_file_path, path, NULL)) {
LogFailure(path, FAILED_RENAMING, "could not rename temporary file");
base::DeleteFile(tmp_file_path, false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment