diff --git a/logging/BUILD.gn b/logging/BUILD.gn index 2258a7356a..7504d6b9d5 100644 --- a/logging/BUILD.gn +++ b/logging/BUILD.gn @@ -239,6 +239,7 @@ rtc_source_set("rtc_event_log_impl_output") { "../api:libjingle_logging_api", "../rtc_base:checks", "../rtc_base:rtc_base_approved", + "../rtc_base/system:file_wrapper", ] } diff --git a/logging/rtc_event_log/output/rtc_event_log_output_file.cc b/logging/rtc_event_log/output/rtc_event_log_output_file.cc index 9e51b41226..3f329a4b79 100644 --- a/logging/rtc_event_log/output/rtc_event_log_output_file.cc +++ b/logging/rtc_event_log/output/rtc_event_log_output_file.cc @@ -9,6 +9,7 @@ */ #include +#include #include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" @@ -17,6 +18,17 @@ namespace webrtc { +namespace { + +FileWrapper FileWrapperFromPlatformFile(rtc::PlatformFile platform_file) { + if (platform_file == rtc::kInvalidPlatformFileValue) + return FileWrapper(); + + return FileWrapper(rtc::FdopenPlatformFileForWriting(platform_file)); +} + +} // namespace + // Together with the assumption of no single Write() would ever be called on // an input with length greater-than-or-equal-to (max(size_t) / 2), this // guarantees no overflow of the check for remaining file capacity in Write(). @@ -25,45 +37,33 @@ const size_t RtcEventLogOutputFile::kMaxReasonableFileSize = std::numeric_limits::max() / 2; RtcEventLogOutputFile::RtcEventLogOutputFile(const std::string& file_name) - : RtcEventLogOutputFile(file_name, RtcEventLog::kUnlimitedOutput) {} + : RtcEventLogOutputFile(FileWrapper::OpenWriteOnly(file_name), + RtcEventLog::kUnlimitedOutput) {} RtcEventLogOutputFile::RtcEventLogOutputFile(const std::string& file_name, size_t max_size_bytes) - // Unlike plain fopen, CreatePlatformFile takes care of filename utf8 -> - // wchar conversion on windows. - : RtcEventLogOutputFile(rtc::CreatePlatformFile(file_name), + // Unlike plain fopen, FileWrapper takes care of filename utf8 -> + // wchar conversion on Windows. + : RtcEventLogOutputFile(FileWrapper::OpenWriteOnly(file_name), max_size_bytes) {} -RtcEventLogOutputFile::RtcEventLogOutputFile(rtc::PlatformFile file) - : RtcEventLogOutputFile(file, RtcEventLog::kUnlimitedOutput) {} +RtcEventLogOutputFile::RtcEventLogOutputFile(FILE* file, size_t max_size_bytes) + : RtcEventLogOutputFile(FileWrapper(file), max_size_bytes) {} + +RtcEventLogOutputFile::RtcEventLogOutputFile(FileWrapper file, + size_t max_size_bytes) + : max_size_bytes_(max_size_bytes), file_(std::move(file)) { + RTC_CHECK_LE(max_size_bytes_, kMaxReasonableFileSize); + if (!file_.is_open()) { + RTC_LOG(LS_ERROR) << "Invalid file. WebRTC event log not started."; + } +} RtcEventLogOutputFile::RtcEventLogOutputFile(rtc::PlatformFile platform_file, size_t max_size_bytes) - : max_size_bytes_(max_size_bytes) { - RTC_CHECK_LE(max_size_bytes_, kMaxReasonableFileSize); - - // Handle errors from the CreatePlatformFile call in above constructor. - if (platform_file == rtc::kInvalidPlatformFileValue) { - RTC_LOG(LS_ERROR) << "Invalid file. WebRTC event log not started."; - return; - } - file_ = rtc::FdopenPlatformFileForWriting(platform_file); - if (!file_) { - RTC_LOG(LS_ERROR) << "Can't open file. WebRTC event log not started."; - // Even though we failed to open a FILE*, the file is still open - // and needs to be closed. - if (!rtc::ClosePlatformFile(platform_file)) { - RTC_LOG(LS_ERROR) << "Can't close file."; - } - } -} - -RtcEventLogOutputFile::~RtcEventLogOutputFile() { - if (file_) { - fclose(file_); - } -} + : RtcEventLogOutputFile(FileWrapperFromPlatformFile(platform_file), + max_size_bytes) {} bool RtcEventLogOutputFile::IsActive() const { return IsActiveInternal(); @@ -77,7 +77,7 @@ bool RtcEventLogOutputFile::Write(const std::string& output) { if (max_size_bytes_ == RtcEventLog::kUnlimitedOutput || written_bytes_ + output.length() <= max_size_bytes_) { - if (fwrite(output.c_str(), 1, output.size(), file_) == output.size()) { + if (file_.Write(output.c_str(), output.size())) { written_bytes_ += output.size(); return true; } else { @@ -88,14 +88,13 @@ bool RtcEventLogOutputFile::Write(const std::string& output) { } // Failed, for one of above reasons. Close output file. - fclose(file_); - file_ = nullptr; + file_.Close(); return false; } // Internal non-virtual method. bool RtcEventLogOutputFile::IsActiveInternal() const { - return file_ != nullptr; + return file_.is_open(); } } // namespace webrtc diff --git a/logging/rtc_event_log/output/rtc_event_log_output_file.h b/logging/rtc_event_log/output/rtc_event_log_output_file.h index 5325c5ff16..75a3af6e95 100644 --- a/logging/rtc_event_log/output/rtc_event_log_output_file.h +++ b/logging/rtc_event_log/output/rtc_event_log_output_file.h @@ -17,6 +17,7 @@ #include "api/rtc_event_log_output.h" #include "rtc_base/platform_file.h" // Can't neatly forward PlatformFile. +#include "rtc_base/system/file_wrapper.h" namespace webrtc { @@ -28,17 +29,23 @@ class RtcEventLogOutputFile final : public RtcEventLogOutput { explicit RtcEventLogOutputFile(const std::string& file_name); RtcEventLogOutputFile(const std::string& file_name, size_t max_size_bytes); - // Unlimited/limited-size output file (by file handle). - explicit RtcEventLogOutputFile(rtc::PlatformFile file); + // Limited-size output file (by FILE*). This class takes ownership + // of the FILE*, and closes it on destruction. + RtcEventLogOutputFile(FILE* file, size_t max_size_bytes); + + // TODO(bugs.webrtc.org/6463): Deprecated, delete together with the + // corresponding PeerConnection::StartRtcEventLog override. RtcEventLogOutputFile(rtc::PlatformFile file, size_t max_size_bytes); - ~RtcEventLogOutputFile() override; + ~RtcEventLogOutputFile() override = default; bool IsActive() const override; bool Write(const std::string& output) override; private: + RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes); + // IsActive() can be called either from outside or from inside, but we don't // want to incur the overhead of a virtual function call if called from inside // some other function of this class. @@ -47,7 +54,7 @@ class RtcEventLogOutputFile final : public RtcEventLogOutput { // Maximum size, or zero for no limit. const size_t max_size_bytes_; size_t written_bytes_{0}; - FILE* file_{nullptr}; + FileWrapper file_; }; } // namespace webrtc