From 3baefbf2dda6e65d1c0118abf44f18bc4d62bfaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Terelius?= Date: Tue, 23 Apr 2024 16:51:38 +0200 Subject: [PATCH] Return absl::optional from FileWrapper::FileSize() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: If5219a8f7f7e81ea660b0495c48f96adb6948228 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/348860 Commit-Queue: Björn Terelius Reviewed-by: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#42206} --- logging/rtc_event_log/rtc_event_log_parser.cc | 16 ++++++++-------- rtc_base/system/BUILD.gn | 5 ++++- rtc_base/system/file_wrapper.cc | 13 +++++++------ rtc_base/system/file_wrapper.h | 3 ++- .../analyzer_bindings_unittest.cc | 11 ++++++----- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/logging/rtc_event_log/rtc_event_log_parser.cc b/logging/rtc_event_log/rtc_event_log_parser.cc index 7f1f7e18e6..c41387b8ff 100644 --- a/logging/rtc_event_log/rtc_event_log_parser.cc +++ b/logging/rtc_event_log/rtc_event_log_parser.cc @@ -53,7 +53,7 @@ using webrtc_event_logging::ToUnsigned; namespace webrtc { namespace { -constexpr int64_t kMaxLogSize = 250000000; +constexpr size_t kMaxLogSize = 250000000; constexpr size_t kIpv4Overhead = 20; constexpr size_t kIpv6Overhead = 40; @@ -1127,17 +1127,17 @@ ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::ParseFile( } // Compute file size. - long signed_filesize = file.FileSize(); // NOLINT(runtime/int) - RTC_PARSE_CHECK_OR_RETURN_GE(signed_filesize, 0); - RTC_PARSE_CHECK_OR_RETURN_LE(signed_filesize, kMaxLogSize); - size_t filesize = rtc::checked_cast(signed_filesize); + absl::optional file_size = file.FileSize(); + RTC_PARSE_CHECK_OR_RETURN(file_size.has_value()); + RTC_PARSE_CHECK_OR_RETURN_GE(*file_size, 0u); + RTC_PARSE_CHECK_OR_RETURN_LE(*file_size, kMaxLogSize); // Read file into memory. - std::string buffer(filesize, '\0'); + std::string buffer(*file_size, '\0'); size_t bytes_read = file.Read(&buffer[0], buffer.size()); - if (bytes_read != filesize) { + if (bytes_read != *file_size) { RTC_LOG(LS_WARNING) << "Failed to read file " << filename; - RTC_PARSE_CHECK_OR_RETURN_EQ(bytes_read, filesize); + RTC_PARSE_CHECK_OR_RETURN_EQ(bytes_read, *file_size); } return ParseStream(buffer); diff --git a/rtc_base/system/BUILD.gn b/rtc_base/system/BUILD.gn index 77f5139a2f..4360317d3b 100644 --- a/rtc_base/system/BUILD.gn +++ b/rtc_base/system/BUILD.gn @@ -30,7 +30,10 @@ rtc_library("file_wrapper") { "..:criticalsection", "..:safe_conversions", ] - absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/strings", + "//third_party/abseil-cpp/absl/types:optional", + ] } if (rtc_include_tests) { diff --git a/rtc_base/system/file_wrapper.cc b/rtc_base/system/file_wrapper.cc index af34d0e411..b768ad0417 100644 --- a/rtc_base/system/file_wrapper.cc +++ b/rtc_base/system/file_wrapper.cc @@ -17,6 +17,7 @@ #include #include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "rtc_base/checks.h" #include "rtc_base/numerics/safe_conversions.h" @@ -81,20 +82,20 @@ bool FileWrapper::SeekTo(int64_t position) { return fseek(file_, rtc::checked_cast(position), SEEK_SET) == 0; } -long FileWrapper::FileSize() { +absl::optional FileWrapper::FileSize() { if (file_ == nullptr) - return -1; + return absl::nullopt; long original_position = ftell(file_); if (original_position < 0) - return -1; + return absl::nullopt; int seek_error = fseek(file_, 0, SEEK_END); if (seek_error) - return -1; + return absl::nullopt; long file_size = ftell(file_); seek_error = fseek(file_, original_position, SEEK_SET); if (seek_error) - return -1; - return file_size; + return absl::nullopt; + return rtc::checked_cast(file_size); } bool FileWrapper::Flush() { diff --git a/rtc_base/system/file_wrapper.h b/rtc_base/system/file_wrapper.h index 92a552cfd9..882393bbf3 100644 --- a/rtc_base/system/file_wrapper.h +++ b/rtc_base/system/file_wrapper.h @@ -18,6 +18,7 @@ #include #include "absl/strings/string_view.h" +#include "absl/types/optional.h" // Implementation that can read (exclusive) or write from/to a file. @@ -89,7 +90,7 @@ class FileWrapper final { // Returns the file size or -1 if a size could not be determined. // (A file size might not exists for non-seekable files or file-like // objects, for example /dev/tty on unix.) - long FileSize(); + absl::optional FileSize(); // Returns number of bytes read. Short count indicates EOF or error. size_t Read(void* buf, size_t length); diff --git a/rtc_tools/rtc_event_log_visualizer/analyzer_bindings_unittest.cc b/rtc_tools/rtc_event_log_visualizer/analyzer_bindings_unittest.cc index feb8104297..99ceccc69b 100644 --- a/rtc_tools/rtc_event_log_visualizer/analyzer_bindings_unittest.cc +++ b/rtc_tools/rtc_event_log_visualizer/analyzer_bindings_unittest.cc @@ -37,11 +37,12 @@ TEST(RtcEventLogAnalyzerBindingsTest, ProducesCharts) { "rtc_event_log/rtc_event_log_500kbps", "binarypb"); webrtc::FileWrapper file = webrtc::FileWrapper::OpenReadOnly(file_name); ASSERT_TRUE(file.is_open()); - int64_t file_size = file.FileSize(); - ASSERT_LE(file_size, kInputBufferSize); - ASSERT_GT(file_size, 0); - size_t input_size = file.Read(input.get(), static_cast(file_size)); - ASSERT_EQ(static_cast(file_size), input_size); + absl::optional file_size = file.FileSize(); + ASSERT_TRUE(file_size.has_value()); + ASSERT_LE(*file_size, static_cast(kInputBufferSize)); + ASSERT_GT(*file_size, 0u); + size_t input_size = file.Read(input.get(), *file_size); + ASSERT_EQ(*file_size, input_size); // Call analyzer. uint32_t output_size = kOutputBufferSize;