Trim down FileWrapper class to be merely a wrapper owning a FILE*
Bug: webrtc:6463
Change-Id: If71e2f3a75dc1863bc805ab71de1e2d33294f805
Reviewed-on: https://webrtc-review.googlesource.com/c/117881
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26311}diff --git a/rtc_base/system/file_wrapper.cc b/rtc_base/system/file_wrapper.cc
index c033a79..dbea1ca 100644
--- a/rtc_base/system/file_wrapper.cc
+++ b/rtc_base/system/file_wrapper.cc
@@ -34,22 +34,13 @@
} // namespace
// static
-FileWrapper* FileWrapper::Create() {
- return new FileWrapper();
+FileWrapper FileWrapper::OpenReadOnly(const char* file_name_utf8) {
+ return FileWrapper(FileOpen(file_name_utf8, true));
}
// static
-FileWrapper FileWrapper::Open(const char* file_name_utf8, bool read_only) {
- return FileWrapper(FileOpen(file_name_utf8, read_only), 0);
-}
-
-FileWrapper::FileWrapper() {}
-
-FileWrapper::FileWrapper(FILE* file, size_t max_size)
- : file_(file), max_size_in_bytes_(max_size) {}
-
-FileWrapper::~FileWrapper() {
- CloseFileImpl();
+FileWrapper FileWrapper::OpenWriteOnly(const char* file_name_utf8) {
+ return FileWrapper(FileOpen(file_name_utf8, false));
}
FileWrapper::FileWrapper(FileWrapper&& other) {
@@ -57,95 +48,39 @@
}
FileWrapper& FileWrapper::operator=(FileWrapper&& other) {
+ Close();
file_ = other.file_;
- max_size_in_bytes_ = other.max_size_in_bytes_;
- position_ = other.position_;
other.file_ = nullptr;
return *this;
}
-void FileWrapper::CloseFile() {
- rtc::CritScope lock(&lock_);
- CloseFileImpl();
+bool FileWrapper::Rewind() {
+ RTC_DCHECK(file_);
+ return fseek(file_, 0, SEEK_SET) == 0;
}
-int FileWrapper::Rewind() {
- rtc::CritScope lock(&lock_);
- if (file_ != nullptr) {
- position_ = 0;
- return fseek(file_, 0, SEEK_SET);
- }
- return -1;
+bool FileWrapper::Flush() {
+ RTC_DCHECK(file_);
+ return fflush(file_) == 0;
}
-void FileWrapper::SetMaxFileSize(size_t bytes) {
- rtc::CritScope lock(&lock_);
- max_size_in_bytes_ = bytes;
-}
-
-int FileWrapper::Flush() {
- rtc::CritScope lock(&lock_);
- return FlushImpl();
-}
-
-bool FileWrapper::OpenFile(const char* file_name_utf8, bool read_only) {
- size_t length = strlen(file_name_utf8);
- if (length > kMaxFileNameSize - 1)
- return false;
-
- rtc::CritScope lock(&lock_);
- if (file_ != nullptr)
- return false;
-
- file_ = FileOpen(file_name_utf8, read_only);
- return file_ != nullptr;
-}
-
-bool FileWrapper::OpenFromFileHandle(FILE* handle) {
- if (!handle)
- return false;
- rtc::CritScope lock(&lock_);
- CloseFileImpl();
- file_ = handle;
- return true;
-}
-
-int FileWrapper::Read(void* buf, size_t length) {
- rtc::CritScope lock(&lock_);
- if (file_ == nullptr)
- return -1;
-
- size_t bytes_read = fread(buf, 1, length, file_);
- return static_cast<int>(bytes_read);
+size_t FileWrapper::Read(void* buf, size_t length) {
+ RTC_DCHECK(file_);
+ return fread(buf, 1, length, file_);
}
bool FileWrapper::Write(const void* buf, size_t length) {
- if (buf == nullptr)
- return false;
+ RTC_DCHECK(file_);
+ return fwrite(buf, 1, length, file_) == length;
+}
- rtc::CritScope lock(&lock_);
-
+bool FileWrapper::Close() {
if (file_ == nullptr)
- return false;
+ return true;
- // Check if it's time to stop writing.
- if (max_size_in_bytes_ > 0 && (position_ + length) > max_size_in_bytes_)
- return false;
-
- size_t num_bytes = fwrite(buf, 1, length, file_);
- position_ += num_bytes;
-
- return num_bytes == length;
-}
-
-void FileWrapper::CloseFileImpl() {
- if (file_ != nullptr)
- fclose(file_);
+ bool success = fclose(file_) == 0;
file_ = nullptr;
-}
-
-int FileWrapper::FlushImpl() {
- return (file_ != nullptr) ? fflush(file_) : -1;
+ return success;
}
} // namespace webrtc
diff --git a/rtc_base/system/file_wrapper.h b/rtc_base/system/file_wrapper.h
index f2ed51a..d56e131 100644
--- a/rtc_base/system/file_wrapper.h
+++ b/rtc_base/system/file_wrapper.h
@@ -20,64 +20,57 @@
namespace webrtc {
-// TODO(tommi): Rename to rtc::File and move to base.
class FileWrapper final {
public:
- static const size_t kMaxFileNameSize = 1024;
+ // Opens a file, in read or write mode. Use the is_open() method on the
+ // returned object to check if the open operation was successful. The file is
+ // closed by the destructor.
+ static FileWrapper OpenReadOnly(const char* file_name_utf8);
+ static FileWrapper OpenWriteOnly(const char* file_name_utf8);
- // Factory methods.
- // TODO(tommi): Remove Create().
- static FileWrapper* Create();
- static FileWrapper Open(const char* file_name_utf8, bool read_only);
+ FileWrapper() = default;
- FileWrapper(FILE* file, size_t max_size);
- ~FileWrapper();
-
- // Support for move semantics.
- FileWrapper(FileWrapper&& other);
- FileWrapper& operator=(FileWrapper&& other);
-
- // Returns true if a file has been opened.
- bool is_open() const { return file_ != nullptr; }
-
- // Opens a file in read or write mode, decided by the read_only parameter.
- bool OpenFile(const char* file_name_utf8, bool read_only);
-
- // Initializes the wrapper from an existing handle. The wrapper
- // takes ownership of |handle| and closes it in CloseFile().
- bool OpenFromFileHandle(FILE* handle);
-
- void CloseFile();
-
- // Limits the file size to |bytes|. Writing will fail after the cap
- // is hit. Pass zero to use an unlimited size.
- // TODO(tommi): Could we move this out into a separate class?
- void SetMaxFileSize(size_t bytes);
-
- // Flush any pending writes. Note: Flushing when closing, is not required.
- int Flush();
-
- // Rewinds the file to the start.
- int Rewind();
- int Read(void* buf, size_t length);
- bool Write(const void* buf, size_t length);
-
- private:
- FileWrapper();
-
- void CloseFileImpl();
- int FlushImpl();
-
- // TODO(tommi): Remove the lock.
- rtc::CriticalSection lock_;
-
- FILE* file_ = nullptr;
- size_t position_ = 0;
- size_t max_size_in_bytes_ = 0;
+ // Takes over ownership of |file|, closing it on destruction.
+ explicit FileWrapper(FILE* file) : file_(file) {}
+ ~FileWrapper() { Close(); }
// Copying is not supported.
FileWrapper(const FileWrapper&) = delete;
FileWrapper& operator=(const FileWrapper&) = delete;
+
+ // Support for move semantics.
+ FileWrapper(FileWrapper&&);
+ FileWrapper& operator=(FileWrapper&&);
+
+ // Returns true if a file has been opened. If the file is not open, no methods
+ // but is_open and Close may be called.
+ bool is_open() const { return file_ != nullptr; }
+
+ // Closes the file, and implies Flush. Returns true on success, false if
+ // writing buffered data fails. On failure, the file is nevertheless closed.
+ // Calling Close on an already closed file does nothing and returns success.
+ bool Close();
+
+ // Write any buffered data to the underlying file. Returns true on success,
+ // false on write error. Note: Flushing when closing, is not required.
+ // TODO(nisse): Delete this method.
+ bool Flush();
+
+ // Seeks to the beginning of file. Returns true on success, false on failure,
+ // e.g., if the underlying file isn't seekable.
+ // TODO(nisse): Delete this method.
+ bool Rewind();
+
+ // Returns number of bytes read. Short count indicates EOF or error.
+ size_t Read(void* buf, size_t length);
+
+ // Returns true if all data was successfully written (or buffered), or false
+ // if there was an error. Writing buffered data can fail later, and is
+ // reported with return value from Flush or Close.
+ bool Write(const void* buf, size_t length);
+
+ private:
+ FILE* file_ = nullptr;
};
} // namespace webrtc