From d86071d783630393578046e389850953501df3ac Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 9 May 2017 12:02:12 -0700 Subject: [PATCH] Fix possible corruption of ktx cache files When creating a file like this: 1. Create file 2. Write file 3. Close file there is an opening before the file is written and closed where, if the application were to crash (or not finish for any other reason), the file would be left in an undefined state. Instead, this change updates it to do this: 1. Create temp file 2. Write temp file 3. Close temp file 4. Move temp file to final path This ensures that at step 3 we have a valid file, before we rename it. We should never end up with a final file in an undefined state, but it is possible to end up with a temp file that is an undefined state if we, for instance, crash during step 2. --- libraries/networking/src/FileCache.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 0a859d511b..0ceda7a745 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -110,10 +110,16 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { return file; } - // write the new file - FILE* saveFile = fopen(filepath.c_str(), "wb"); + // write the data to a temporary file + std::string tmpFilepath = filepath + "_tmp"; + FILE* saveFile = fopen(tmpFilepath.c_str(), "wb"); if (saveFile != nullptr && fwrite(data, metadata.length, 1, saveFile) && fclose(saveFile) == 0) { - file = addFile(std::move(metadata), filepath); + int result = rename(tmpFilepath.c_str(), filepath.c_str()); + if (result == 0) { + file = addFile(std::move(metadata), filepath); + } else { + qCWarning(file_cache, "[%s] Failed to rename %s to %s (%s)", tmpFilepath.c_str(), filepath.c_str(), strerror(errno)); + } } else { qCWarning(file_cache, "[%s] Failed to write %s (%s)", _dirname.c_str(), metadata.key.c_str(), strerror(errno)); errno = 0;