diff --git a/rtc_base/filerotatingstream.cc b/rtc_base/filerotatingstream.cc index 09f7faab2a..84227b9a07 100644 --- a/rtc_base/filerotatingstream.cc +++ b/rtc_base/filerotatingstream.cc @@ -169,65 +169,31 @@ absl::optional GetFileSize(const std::string& file) { } // namespace -FileRotatingStream::FileRotatingStream(const std::string& dir_path, - const std::string& file_prefix) - : FileRotatingStream(dir_path, file_prefix, 0, 0, kRead) {} - FileRotatingStream::FileRotatingStream(const std::string& dir_path, const std::string& file_prefix, size_t max_file_size, size_t num_files) - : FileRotatingStream(dir_path, - file_prefix, - max_file_size, - num_files, - kWrite) { - RTC_DCHECK_GT(max_file_size, 0); - RTC_DCHECK_GT(num_files, 1); -} - -FileRotatingStream::FileRotatingStream(const std::string& dir_path, - const std::string& file_prefix, - size_t max_file_size, - size_t num_files, - Mode mode) : dir_path_(AddTrailingPathDelimiterIfNeeded(dir_path)), file_prefix_(file_prefix), - mode_(mode), file_stream_(nullptr), max_file_size_(max_file_size), current_file_index_(0), rotation_index_(0), current_bytes_written_(0), disable_buffering_(false) { + RTC_DCHECK_GT(max_file_size, 0); + RTC_DCHECK_GT(num_files, 1); RTC_DCHECK(IsFolder(dir_path)); - switch (mode) { - case kWrite: { - file_names_.clear(); - for (size_t i = 0; i < num_files; ++i) { - file_names_.push_back(GetFilePath(i, num_files)); - } - rotation_index_ = num_files - 1; - break; - } - case kRead: { - file_names_ = GetFilesWithPrefix(dir_path_, file_prefix_); - std::sort(file_names_.begin(), file_names_.end()); - if (file_names_.size() > 0) { - // |file_names_| is sorted newest first, so read from the end. - current_file_index_ = file_names_.size() - 1; - } - break; - } + file_names_.clear(); + for (size_t i = 0; i < num_files; ++i) { + file_names_.push_back(GetFilePath(i, num_files)); } + rotation_index_ = num_files - 1; } FileRotatingStream::~FileRotatingStream() {} StreamState FileRotatingStream::GetState() const { - if (mode_ == kRead && current_file_index_ < file_names_.size()) { - return SS_OPEN; - } if (!file_stream_) { return SS_CLOSED; } @@ -239,59 +205,14 @@ StreamResult FileRotatingStream::Read(void* buffer, size_t* read, int* error) { RTC_DCHECK(buffer); - if (mode_ != kRead) { - return SR_EOS; - } - if (current_file_index_ >= file_names_.size()) { - return SR_EOS; - } - // We will have no file stream initially, and when we are finished with the - // previous file. - if (!file_stream_) { - if (!OpenCurrentFile()) { - return SR_ERROR; - } - } - int local_error = 0; - if (!error) { - error = &local_error; - } - StreamResult result = file_stream_->Read(buffer, buffer_len, read, error); - if (result == SR_EOS || result == SR_ERROR) { - if (result == SR_ERROR) { - RTC_LOG(LS_ERROR) << "Failed to read from: " - << file_names_[current_file_index_] - << "Error: " << error; - } - // Reached the end of the file, read next file. If there is an error return - // the error status but allow for a next read by reading next file. - CloseCurrentFile(); - if (current_file_index_ == 0) { - // Just finished reading the last file, signal EOS by setting index. - current_file_index_ = file_names_.size(); - } else { - --current_file_index_; - } - if (read) { - *read = 0; - } - return result == SR_EOS ? SR_SUCCESS : result; - } else if (result == SR_SUCCESS) { - // Succeeded, continue reading from this file. - return SR_SUCCESS; - } else { - RTC_NOTREACHED(); - } - return result; + RTC_NOTREACHED(); + return SR_EOS; } StreamResult FileRotatingStream::Write(const void* data, size_t data_len, size_t* written, int* error) { - if (mode_ != kWrite) { - return SR_EOS; - } if (!file_stream_) { std::fprintf(stderr, "Open() must be called before Write.\n"); return SR_ERROR; @@ -323,19 +244,9 @@ bool FileRotatingStream::Flush() { } bool FileRotatingStream::GetSize(size_t* size) const { - if (mode_ != kRead) { - // Not possible to get accurate size on disk when writing because of - // potential buffering. - return false; - } - RTC_DCHECK(size); - *size = 0; - size_t total_size = 0; - for (auto file_name : file_names_) { - total_size += GetFileSize(file_name).value_or(0); - } - *size = total_size; - return true; + // Not possible to get accurate size on disk when writing because of + // potential buffering. + return false; } void FileRotatingStream::Close() { @@ -343,24 +254,15 @@ void FileRotatingStream::Close() { } bool FileRotatingStream::Open() { - switch (mode_) { - case kRead: - // Defer opening to when we first read since we want to return read error - // if we fail to open next file. - return true; - case kWrite: { - // Delete existing files when opening for write. - std::vector matching_files = - GetFilesWithPrefix(dir_path_, file_prefix_); - for (const auto& matching_file : matching_files) { - if (!DeleteFile(matching_file)) { - std::fprintf(stderr, "Failed to delete: %s\n", matching_file.c_str()); - } - } - return OpenCurrentFile(); + // Delete existing files when opening for write. + std::vector matching_files = + GetFilesWithPrefix(dir_path_, file_prefix_); + for (const auto& matching_file : matching_files) { + if (!DeleteFile(matching_file)) { + std::fprintf(stderr, "Failed to delete: %s\n", matching_file.c_str()); } } - return false; + return OpenCurrentFile(); } bool FileRotatingStream::DisableBuffering() { @@ -384,19 +286,11 @@ bool FileRotatingStream::OpenCurrentFile() { RTC_DCHECK_LT(current_file_index_, file_names_.size()); std::string file_path = file_names_[current_file_index_]; file_stream_.reset(new FileStream()); - const char* mode = nullptr; - switch (mode_) { - case kWrite: - mode = "w+"; - // We should always we writing to the zero-th file. - RTC_DCHECK_EQ(current_file_index_, 0); - break; - case kRead: - mode = "r"; - break; - } + + // We should always be writing to the zero-th file. + RTC_DCHECK_EQ(current_file_index_, 0); int error = 0; - if (!file_stream_->Open(file_path, mode, &error)) { + if (!file_stream_->Open(file_path, "w+", &error)) { std::fprintf(stderr, "Failed to open: %s Error: %i\n", file_path.c_str(), error); file_stream_.reset(); @@ -417,7 +311,6 @@ void FileRotatingStream::CloseCurrentFile() { } void FileRotatingStream::RotateFiles() { - RTC_DCHECK_EQ(mode_, kWrite); CloseCurrentFile(); // Rotates the files by deleting the file at |rotation_index_|, which is the // oldest file and then renaming the newer files to have an incremented index. @@ -458,12 +351,6 @@ std::string FileRotatingStream::GetFilePath(size_t index, return dir_path_ + file_prefix_ + file_postfix; } -CallSessionFileRotatingStream::CallSessionFileRotatingStream( - const std::string& dir_path) - : FileRotatingStream(dir_path, kCallSessionLogPrefix), - max_total_log_size_(0), - num_rotations_(0) {} - CallSessionFileRotatingStream::CallSessionFileRotatingStream( const std::string& dir_path, size_t max_total_log_size) diff --git a/rtc_base/filerotatingstream.h b/rtc_base/filerotatingstream.h index a06efdf8e4..48cf727cfc 100644 --- a/rtc_base/filerotatingstream.h +++ b/rtc_base/filerotatingstream.h @@ -83,14 +83,6 @@ class FileRotatingStream : public StreamInterface { virtual void OnRotation() {} private: - enum Mode { kRead, kWrite }; - - FileRotatingStream(const std::string& dir_path, - const std::string& file_prefix, - size_t max_file_size, - size_t num_files, - Mode mode); - bool OpenCurrentFile(); void CloseCurrentFile(); @@ -107,7 +99,6 @@ class FileRotatingStream : public StreamInterface { const std::string dir_path_; const std::string file_prefix_; - const Mode mode_; // FileStream is used to write to the current file. std::unique_ptr file_stream_; @@ -127,7 +118,7 @@ class FileRotatingStream : public StreamInterface { }; // CallSessionFileRotatingStream is meant to be used in situations where we will -// have limited disk space. Its purpose is to read and write logs up to a +// have limited disk space. Its purpose is to write logs up to a // maximum size. Once the maximum size is exceeded, logs from the middle are // deleted whereas logs from the beginning and end are preserved. The reason for // this is because we anticipate that in WebRTC the beginning and end of the @@ -140,11 +131,11 @@ class FileRotatingStream : public StreamInterface { // (earliest) file on rotate, and that that file's size is bigger. // // Open() must be called before using this stream. + +// To read the logs produced by this class, one can use the companion class +// CallSessionFileRotatingStreamReader. class CallSessionFileRotatingStream : public FileRotatingStream { public: - // Use this constructor for reading a directory previously written to with - // this stream. - explicit CallSessionFileRotatingStream(const std::string& dir_path); // Use this constructor for writing to a directory. Files in the directory // matching what's used by the stream will be deleted. |max_total_log_size| // must be at least 4. diff --git a/rtc_base/filerotatingstream_unittest.cc b/rtc_base/filerotatingstream_unittest.cc index 07a3e1ab25..dc23e34fc4 100644 --- a/rtc_base/filerotatingstream_unittest.cc +++ b/rtc_base/filerotatingstream_unittest.cc @@ -80,24 +80,9 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test { const size_t expected_length, const std::string& dir_path, const char* file_prefix) { - // TODO(nisse): Delete the part of this method using read-mode - // FileRotatingStream, together with support for read-mode. - std::unique_ptr stream; - stream.reset(new FileRotatingStream(dir_path, file_prefix)); - ASSERT_TRUE(stream->Open()); - size_t read = 0; - size_t stream_size = 0; - EXPECT_TRUE(stream->GetSize(&stream_size)); - std::unique_ptr buffer(new uint8_t[expected_length]); - EXPECT_EQ(SR_SUCCESS, - stream->ReadAll(buffer.get(), expected_length, &read, nullptr)); - EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length)); - EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr)); - EXPECT_EQ(stream_size, read); - - // Test also with the FileRotatingStreamReader class. FileRotatingStreamReader reader(dir_path, file_prefix); EXPECT_EQ(reader.GetSize(), expected_length); + std::unique_ptr buffer(new uint8_t[expected_length]); memset(buffer.get(), 0, expected_length); EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length)); EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length)); @@ -109,9 +94,12 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test { std::unique_ptr buffer(new uint8_t[expected_length]); FileStream stream; ASSERT_TRUE(stream.Open(file_path, "r", nullptr)); - EXPECT_EQ(rtc::SR_SUCCESS, - stream.ReadAll(buffer.get(), expected_length, nullptr, nullptr)); - EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length)); + size_t size_read = 0; + EXPECT_EQ(rtc::SR_SUCCESS, stream.ReadAll(buffer.get(), expected_length, + &size_read, nullptr)); + EXPECT_EQ(size_read, expected_length); + EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), + std::min(expected_length, size_read))); size_t file_size = 0; EXPECT_TRUE(stream.GetSize(&file_size)); EXPECT_EQ(file_size, expected_length); @@ -302,24 +290,9 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test { void VerifyStreamRead(const char* expected_contents, const size_t expected_length, const std::string& dir_path) { - // TODO(nisse): Delete the part of this method using read-mode - // CallSessionFileRotatingStream, together with support for read-mode. - std::unique_ptr stream( - new CallSessionFileRotatingStream(dir_path)); - ASSERT_TRUE(stream->Open()); - size_t read = 0; - size_t stream_size = 0; - EXPECT_TRUE(stream->GetSize(&stream_size)); - std::unique_ptr buffer(new uint8_t[expected_length]); - EXPECT_EQ(SR_SUCCESS, - stream->ReadAll(buffer.get(), expected_length, &read, nullptr)); - EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length)); - EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr)); - EXPECT_EQ(stream_size, read); - - // Test also with the CallSessionFileRotatingStreamReader class. CallSessionFileRotatingStreamReader reader(dir_path); EXPECT_EQ(reader.GetSize(), expected_length); + std::unique_ptr buffer(new uint8_t[expected_length]); memset(buffer.get(), 0, expected_length); EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length)); EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length)); @@ -369,17 +342,25 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadLarge) { stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr)); } - stream_.reset(new CallSessionFileRotatingStream(dir_path_)); - ASSERT_TRUE(stream_->Open()); - std::unique_ptr expected_buffer(new uint8_t[buffer_size]); - int expected_vals[] = {0, 1, 2, 6, 7}; + const int expected_vals[] = {0, 1, 2, 6, 7}; + const size_t expected_size = buffer_size * arraysize(expected_vals); + + CallSessionFileRotatingStreamReader reader(dir_path_); + EXPECT_EQ(reader.GetSize(), expected_size); + std::unique_ptr contents(new uint8_t[expected_size + 1]); + EXPECT_EQ(reader.ReadAll(contents.get(), expected_size + 1), expected_size); for (size_t i = 0; i < arraysize(expected_vals); ++i) { - memset(expected_buffer.get(), expected_vals[i], buffer_size); - EXPECT_EQ(SR_SUCCESS, - stream_->ReadAll(buffer.get(), buffer_size, nullptr, nullptr)); - EXPECT_EQ(0, memcmp(buffer.get(), expected_buffer.get(), buffer_size)); + const uint8_t* block = contents.get() + i * buffer_size; + bool match = true; + for (size_t j = 0; j < buffer_size; j++) { + if (block[j] != expected_vals[i]) { + match = false; + break; + } + } + // EXPECT call at end of loop, to limit the number of messages on failure. + EXPECT_TRUE(match); } - EXPECT_EQ(SR_EOS, stream_->ReadAll(buffer.get(), 1, nullptr, nullptr)); } // Tests that writing and reading to a stream where only the first file is @@ -396,17 +377,26 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadFirstHalf) { stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr)); } - stream_.reset(new CallSessionFileRotatingStream(dir_path_)); - ASSERT_TRUE(stream_->Open()); - std::unique_ptr expected_buffer(new uint8_t[buffer_size]); - int expected_vals[] = {0, 1}; + const int expected_vals[] = {0, 1}; + const size_t expected_size = buffer_size * arraysize(expected_vals); + + CallSessionFileRotatingStreamReader reader(dir_path_); + EXPECT_EQ(reader.GetSize(), expected_size); + std::unique_ptr contents(new uint8_t[expected_size + 1]); + EXPECT_EQ(reader.ReadAll(contents.get(), expected_size + 1), expected_size); + for (size_t i = 0; i < arraysize(expected_vals); ++i) { - memset(expected_buffer.get(), expected_vals[i], buffer_size); - EXPECT_EQ(SR_SUCCESS, - stream_->ReadAll(buffer.get(), buffer_size, nullptr, nullptr)); - EXPECT_EQ(0, memcmp(buffer.get(), expected_buffer.get(), buffer_size)); + const uint8_t* block = contents.get() + i * buffer_size; + bool match = true; + for (size_t j = 0; j < buffer_size; j++) { + if (block[j] != expected_vals[i]) { + match = false; + break; + } + } + // EXPECT call at end of loop, to limit the number of messages on failure. + EXPECT_TRUE(match); } - EXPECT_EQ(SR_EOS, stream_->ReadAll(buffer.get(), 1, nullptr, nullptr)); } } // namespace rtc