diff --git a/rtc_base/filerotatingstream.cc b/rtc_base/filerotatingstream.cc index 325c6bbab0..09f7faab2a 100644 --- a/rtc_base/filerotatingstream.cc +++ b/rtc_base/filerotatingstream.cc @@ -36,6 +36,8 @@ namespace rtc { namespace { +const char kCallSessionLogPrefix[] = "webrtc_log"; + std::string AddTrailingPathDelimiterIfNeeded(std::string directory); // |dir| must have a trailing delimiter. |prefix| must not include wild card @@ -458,7 +460,7 @@ std::string FileRotatingStream::GetFilePath(size_t index, CallSessionFileRotatingStream::CallSessionFileRotatingStream( const std::string& dir_path) - : FileRotatingStream(dir_path, kLogPrefix), + : FileRotatingStream(dir_path, kCallSessionLogPrefix), max_total_log_size_(0), num_rotations_(0) {} @@ -466,7 +468,7 @@ CallSessionFileRotatingStream::CallSessionFileRotatingStream( const std::string& dir_path, size_t max_total_log_size) : FileRotatingStream(dir_path, - kLogPrefix, + kCallSessionLogPrefix, max_total_log_size / 2, GetNumRotatingLogFiles(max_total_log_size) + 1), max_total_log_size_(max_total_log_size), @@ -474,7 +476,6 @@ CallSessionFileRotatingStream::CallSessionFileRotatingStream( RTC_DCHECK_GE(max_total_log_size, 4); } -const char* CallSessionFileRotatingStream::kLogPrefix = "webrtc_log"; const size_t CallSessionFileRotatingStream::kRotatingLogFileDefaultSize = 1024 * 1024; @@ -508,4 +509,47 @@ size_t CallSessionFileRotatingStream::GetNumRotatingLogFiles( (max_total_log_size / 2) / kRotatingLogFileDefaultSize); } +FileRotatingStreamReader::FileRotatingStreamReader( + const std::string& dir_path, + const std::string& file_prefix) { + file_names_ = GetFilesWithPrefix(AddTrailingPathDelimiterIfNeeded(dir_path), + file_prefix); + + // Plain sort of the file names would sort by age, i.e., oldest last. Using + // std::greater gives us the desired chronological older, oldest first. + std::sort(file_names_.begin(), file_names_.end(), + std::greater()); +} + +FileRotatingStreamReader::~FileRotatingStreamReader() = default; + +size_t FileRotatingStreamReader::GetSize() const { + size_t total_size = 0; + for (const auto& file_name : file_names_) { + total_size += GetFileSize(file_name).value_or(0); + } + return total_size; +} + +size_t FileRotatingStreamReader::ReadAll(void* buffer, size_t size) const { + size_t done = 0; + for (const auto& file_name : file_names_) { + if (done < size) { + FILE* f = fopen(file_name.c_str(), "rb"); + if (!f) { + break; + } + done += fread(static_cast(buffer) + done, 1, size - done, f); + fclose(f); + } else { + break; + } + } + return done; +} + +CallSessionFileRotatingStreamReader::CallSessionFileRotatingStreamReader( + const std::string& dir_path) + : FileRotatingStreamReader(dir_path, kCallSessionLogPrefix) {} + } // namespace rtc diff --git a/rtc_base/filerotatingstream.h b/rtc_base/filerotatingstream.h index c75ee153b6..a06efdf8e4 100644 --- a/rtc_base/filerotatingstream.h +++ b/rtc_base/filerotatingstream.h @@ -158,7 +158,6 @@ class CallSessionFileRotatingStream : public FileRotatingStream { private: static size_t GetRotatingLogSize(size_t max_total_log_size); static size_t GetNumRotatingLogFiles(size_t max_total_log_size); - static const char* kLogPrefix; static const size_t kRotatingLogFileDefaultSize; const size_t max_total_log_size_; @@ -167,6 +166,27 @@ class CallSessionFileRotatingStream : public FileRotatingStream { RTC_DISALLOW_COPY_AND_ASSIGN(CallSessionFileRotatingStream); }; +// This is a convenience class, to read all files produced by a +// FileRotatingStream, all in one go. Typical use calls GetSize and ReadData +// only once. The list of file names to read is based on the contents of the log +// directory at construction time. +class FileRotatingStreamReader { + public: + FileRotatingStreamReader(const std::string& dir_path, + const std::string& file_prefix); + ~FileRotatingStreamReader(); + size_t GetSize() const; + size_t ReadAll(void* buffer, size_t size) const; + + private: + std::vector file_names_; +}; + +class CallSessionFileRotatingStreamReader : public FileRotatingStreamReader { + public: + CallSessionFileRotatingStreamReader(const std::string& dir_path); +}; + } // namespace rtc #endif // RTC_BASE_FILEROTATINGSTREAM_H_ diff --git a/rtc_base/filerotatingstream_unittest.cc b/rtc_base/filerotatingstream_unittest.cc index bf52aa35a0..07a3e1ab25 100644 --- a/rtc_base/filerotatingstream_unittest.cc +++ b/rtc_base/filerotatingstream_unittest.cc @@ -80,6 +80,8 @@ 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()); @@ -92,6 +94,13 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test { 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); + 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)); } void VerifyFileContents(const char* expected_contents, @@ -293,6 +302,8 @@ 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()); @@ -305,6 +316,13 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test { 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); + 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)); } std::unique_ptr stream_; diff --git a/sdk/android/src/jni/pc/callsessionfilerotatinglogsink.cc b/sdk/android/src/jni/pc/callsessionfilerotatinglogsink.cc index b3ab2e27b2..d933daa519 100644 --- a/sdk/android/src/jni/pc/callsessionfilerotatinglogsink.cc +++ b/sdk/android/src/jni/pc/callsessionfilerotatinglogsink.cc @@ -50,23 +50,17 @@ JNI_CallSessionFileRotatingLogSink_GetLogData( JNIEnv* jni, const JavaParamRef& j_dirPath) { std::string dir_path = JavaToStdString(jni, j_dirPath); - std::unique_ptr stream( - new rtc::CallSessionFileRotatingStream(dir_path)); - if (!stream->Open()) { - RTC_LOG_V(rtc::LoggingSeverity::LS_WARNING) - << "Failed to open CallSessionFileRotatingStream for path " << dir_path; - return ScopedJavaLocalRef(jni, jni->NewByteArray(0)); - } - size_t log_size = 0; - if (!stream->GetSize(&log_size) || log_size == 0) { + rtc::CallSessionFileRotatingStreamReader file_reader(dir_path); + size_t log_size = file_reader.GetSize(); + if (log_size == 0) { RTC_LOG_V(rtc::LoggingSeverity::LS_WARNING) << "CallSessionFileRotatingStream returns 0 size for path " << dir_path; return ScopedJavaLocalRef(jni, jni->NewByteArray(0)); } - size_t read = 0; + // TODO(nisse, sakal): To avoid copying, change api to use ByteBuffer. std::unique_ptr buffer(static_cast(malloc(log_size))); - stream->ReadAll(buffer.get(), log_size, &read, nullptr); + size_t read = file_reader.ReadAll(buffer.get(), log_size); ScopedJavaLocalRef result = ScopedJavaLocalRef(jni, jni->NewByteArray(read)); diff --git a/sdk/objc/api/peerconnection/RTCFileLogger.mm b/sdk/objc/api/peerconnection/RTCFileLogger.mm index 3a7144af4c..9515cfb813 100644 --- a/sdk/objc/api/peerconnection/RTCFileLogger.mm +++ b/sdk/objc/api/peerconnection/RTCFileLogger.mm @@ -129,29 +129,24 @@ const char *kRTCFileLoggerRotatingLogPrefix = "rotating_log"; return nil; } NSMutableData* logData = [NSMutableData data]; - std::unique_ptr stream; + std::unique_ptr stream; switch(_rotationType) { case RTCFileLoggerTypeApp: - stream.reset( - new rtc::FileRotatingStream(_dirPath.UTF8String, - kRTCFileLoggerRotatingLogPrefix)); + stream = absl::make_unique(_dirPath.UTF8String, + kRTCFileLoggerRotatingLogPrefix); break; case RTCFileLoggerTypeCall: - stream.reset(new rtc::CallSessionFileRotatingStream(_dirPath.UTF8String)); + stream = absl::make_unique(_dirPath.UTF8String); break; } - if (!stream->Open()) { + size_t bufferSize = stream->GetSize(); + if (bufferSize == 0) { return logData; } - size_t bufferSize = 0; - if (!stream->GetSize(&bufferSize) || bufferSize == 0) { - return logData; - } - size_t read = 0; // Allocate memory using malloc so we can pass it direcly to NSData without // copying. std::unique_ptr buffer(static_cast(malloc(bufferSize))); - stream->ReadAll(buffer.get(), bufferSize, &read, nullptr); + size_t read = stream->ReadAll(buffer.get(), bufferSize); logData = [[NSMutableData alloc] initWithBytesNoCopy:buffer.release() length:read]; return logData;