diff --git a/packager/file/file.cc b/packager/file/file.cc index 35273a4163..a0f5aa62ac 100644 --- a/packager/file/file.cc +++ b/packager/file/file.cc @@ -9,9 +9,10 @@ #include #include #include -#include "packager/base/files/important_file_writer.h" +#include "packager/base/files/file_util.h" #include "packager/base/logging.h" #include "packager/base/strings/string_piece.h" +#include "packager/file/file_util.h" #include "packager/file/local_file.h" #include "packager/file/memory_file.h" #include "packager/file/threaded_io_file.h" @@ -61,8 +62,21 @@ bool DeleteLocalFile(const char* file_name) { bool WriteLocalFileAtomically(const char* file_name, const std::string& contents) { - return base::ImportantFileWriter::WriteFileAtomically( - base::FilePath::FromUTF8Unsafe(file_name), contents); + const base::FilePath file_path = base::FilePath::FromUTF8Unsafe(file_name); + const std::string dir_name = file_path.DirName().AsUTF8Unsafe(); + std::string temp_file_name; + if (!TempFilePath(dir_name, &temp_file_name)) + return false; + if (!File::WriteStringToFile(temp_file_name.c_str(), contents)) + return false; + base::File::Error replace_file_error = base::File::FILE_OK; + if (!base::ReplaceFile(base::FilePath::FromUTF8Unsafe(temp_file_name), + file_path, &replace_file_error)) { + LOG(ERROR) << "Failed to replace file '" << file_name << "' with '" + << temp_file_name << "', error: " << replace_file_error; + return false; + } + return true; } File* CreateUdpFile(const char* file_name, const char* mode) { @@ -202,23 +216,8 @@ bool File::ReadFileToString(const char* file_name, std::string* contents) { return len == 0; } -bool File::WriteFileAtomically(const char* file_name, - const std::string& contents) { - base::StringPiece real_file_name; - const FileTypeInfo* file_type = GetFileTypeInfo(file_name, &real_file_name); - DCHECK(file_type); - if (file_type->atomic_write_function) - return file_type->atomic_write_function(real_file_name.data(), contents); - - // Provide a default implementation which may not be atomic unfortunately. - - // Skip the warning message for memory files, which is meant for testing - // anyway.. - if (strncmp(file_name, kMemoryFilePrefix, strlen(kMemoryFilePrefix)) != 0) { - LOG(WARNING) << "Writing to " << file_name - << " is not guaranteed to be atomic."; - } - +bool File::WriteStringToFile(const char* file_name, + const std::string& contents) { std::unique_ptr file(File::Open(file_name, "w")); if (!file) { LOG(ERROR) << "Failed to open file " << file_name; @@ -236,9 +235,32 @@ bool File::WriteFileAtomically(const char* file_name, << contents.size() << " bytes."; return false; } + if (!file->Flush()) { + LOG(ERROR) << "Failed to flush the file '" << file_name << "'."; + return false; + } return true; } +bool File::WriteFileAtomically(const char* file_name, + const std::string& contents) { + base::StringPiece real_file_name; + const FileTypeInfo* file_type = GetFileTypeInfo(file_name, &real_file_name); + DCHECK(file_type); + if (file_type->atomic_write_function) + return file_type->atomic_write_function(real_file_name.data(), contents); + + // Provide a default implementation which may not be atomic unfortunately. + + // Skip the warning message for memory files, which is meant for testing + // anyway.. + if (strncmp(file_name, kMemoryFilePrefix, strlen(kMemoryFilePrefix)) != 0) { + LOG(WARNING) << "Writing to " << file_name + << " is not guaranteed to be atomic."; + } + return WriteStringToFile(file_name, contents); +} + bool File::Copy(const char* from_file_name, const char* to_file_name) { std::string content; if (!ReadFileToString(from_file_name, &content)) { diff --git a/packager/file/file.h b/packager/file/file.h index 28c5687850..a429689e38 100644 --- a/packager/file/file.h +++ b/packager/file/file.h @@ -97,12 +97,19 @@ class File { /// The file will be opened and closed in the process. static int64_t GetFileSize(const char* file_name); - /// Read the file into string. + /// Read the contents of a file into string. /// @param file_name is the file to be read. It should be a valid file. /// @param contents[out] points to the output string. Should not be NULL. /// @return true on success, false otherwise. static bool ReadFileToString(const char* file_name, std::string* contents); + /// Writes the data to file. + /// @param file_name is the file to write to. + /// @param contents is the data to write. + /// @return true on success, false otherwise. + static bool WriteStringToFile(const char* file_name, + const std::string& contents); + /// Save `contents` to `file_name` in an atomic manner. /// @param file_name is the destination file name. /// @param contents is the data to be saved. diff --git a/packager/file/file_unittest.cc b/packager/file/file_unittest.cc index 460dd443fb..fa6774f9e6 100644 --- a/packager/file/file_unittest.cc +++ b/packager/file/file_unittest.cc @@ -154,6 +154,15 @@ TEST_F(LocalFileTest, WriteRead) { EXPECT_EQ(data_, read_data); } +TEST_F(LocalFileTest, WriteStringReadString) { + ASSERT_TRUE( + File::WriteStringToFile(local_file_name_no_prefix_.c_str(), data_)); + std::string read_data; + ASSERT_TRUE( + File::ReadFileToString(local_file_name_no_prefix_.c_str(), &read_data)); + EXPECT_EQ(data_, read_data); +} + // There is no easy way to test if a write operation is atomic. This test only // ensures the data is written correctly. TEST_F(LocalFileTest, AtomicWriteRead) { diff --git a/packager/file/file_util.cc b/packager/file/file_util.cc index 7e1ecca715..7eb4e55546 100644 --- a/packager/file/file_util.cc +++ b/packager/file/file_util.cc @@ -10,17 +10,20 @@ #include "packager/base/files/file_path.h" #include "packager/base/files/file_util.h" +#include "packager/base/process/process_handle.h" #include "packager/base/strings/stringprintf.h" #include "packager/base/threading/platform_thread.h" #include "packager/base/time/time.h" namespace shaka { namespace { -// Create a temp file name using process/thread id and current time. +// Create a temp file name using process id, thread id and current time. std::string TempFileName() { + const int32_t pid = static_cast(base::GetCurrentProcId()); const int32_t tid = static_cast(base::PlatformThread::CurrentId()); const int64_t current_time = base::Time::Now().ToInternalValue(); - return base::StringPrintf("packager-tempfile-%x-%" PRIx64, tid, current_time); + return base::StringPrintf("packager-tempfile-%x-%x-%" PRIx64, pid, tid, + current_time); } } // namespace