From dd87d580e8ffe546bd6e22cbd1522ffe76d675a7 Mon Sep 17 00:00:00 2001 From: zijiehe Date: Tue, 6 Dec 2016 15:04:02 -0800 Subject: [PATCH] Add File::Open / Create functions to take an rtc::Pathname When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues, 1. pathutils and flags are not accessible in testsupport. But both of them are useful for the feature. Pathname can help to combine path with filename, while a flag is needed to handle command line parameter. 2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient. After investigating bug webrtc:3806, flags, pathutils and urlencode are removed from rtc_base_approved because of the including of common.h. So I replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags, pathutils and urlencode pairs now can be placed into rtc_base_approved to unblock file.h to include pathutils.h. Please kindly let me know if you have other concerns about this change. BUG=webrtc:3806, webrtc:6732 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng Review-Url: https://codereview.webrtc.org/2533213005 Cr-Commit-Position: refs/heads/master@{#15451} --- webrtc/base/BUILD.gn | 12 +++---- webrtc/base/file.cc | 33 +++++++++++++++++++ webrtc/base/file.h | 7 ++++- webrtc/base/file_unittest.cc | 26 +++++++++++++++ webrtc/base/fileutils.h | 9 +++--- webrtc/base/flags.h | 1 - webrtc/base/pathutils.cc | 4 +-- webrtc/base/pathutils.h | 61 ++---------------------------------- webrtc/base/urlencode.cc | 4 +-- 9 files changed, 81 insertions(+), 76 deletions(-) diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index 2dea2583a8..10ad64ee54 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -126,6 +126,8 @@ rtc_static_library("rtc_base_approved") { "event_tracer.h", "file.cc", "file.h", + "flags.cc", + "flags.h", "format_macros.h", "function_view.h", "ignore_wundef.h", @@ -139,6 +141,8 @@ rtc_static_library("rtc_base_approved") { "onetimeevent.h", "optional.cc", "optional.h", + "pathutils.cc", + "pathutils.h", "platform_file.cc", "platform_file.h", "platform_thread.cc", @@ -179,6 +183,8 @@ rtc_static_library("rtc_base_approved") { "timeutils.h", "trace_event.h", "type_traits.h", + "urlencode.cc", + "urlencode.h", ] if (is_android) { @@ -413,8 +419,6 @@ rtc_static_library("rtc_base") { "filerotatingstream.h", "fileutils.cc", "fileutils.h", - "flags.cc", - "flags.h", "gunit_prod.h", "helpers.cc", "helpers.h", @@ -451,8 +455,6 @@ rtc_static_library("rtc_base") { "opensslidentity.h", "opensslstreamadapter.cc", "opensslstreamadapter.h", - "pathutils.cc", - "pathutils.h", "physicalsocketserver.cc", "physicalsocketserver.h", "proxydetect.cc", @@ -509,8 +511,6 @@ rtc_static_library("rtc_base") { "taskrunner.h", "thread.cc", "thread.h", - "urlencode.cc", - "urlencode.h", ] # TODO(henrike): issue 3307, make rtc_base build with the Chromium default diff --git a/webrtc/base/file.cc b/webrtc/base/file.cc index 74d48177e4..8b5df9e9be 100644 --- a/webrtc/base/file.cc +++ b/webrtc/base/file.cc @@ -10,8 +10,19 @@ #include "webrtc/base/file.h" +#include + namespace rtc { +namespace { + +std::string NormalizePathname(Pathname&& path) { + path.Normalize(); + return path.pathname(); +} + +} // namespace + File::File(PlatformFile file) : file_(file) {} File::File() : file_(kInvalidPlatformFileValue) {} @@ -20,14 +31,36 @@ File::~File() { Close(); } +// static File File::Open(const std::string& path) { return File(OpenPlatformFile(path)); } +// static +File File::Open(Pathname&& path) { + return Open(NormalizePathname(std::move(path))); +} + +// static +File File::Open(const Pathname& path) { + return Open(Pathname(path)); +} + +// static File File::Create(const std::string& path) { return File(CreatePlatformFile(path)); } +// static +File File::Create(Pathname&& path) { + return Create(NormalizePathname(std::move(path))); +} + +// static +File File::Create(const Pathname& path) { + return Create(Pathname(path)); +} + File::File(File&& other) : file_(other.file_) { other.file_ = kInvalidPlatformFileValue; } diff --git a/webrtc/base/file.h b/webrtc/base/file.h index 10fb69834b..5e8449ccdf 100644 --- a/webrtc/base/file.h +++ b/webrtc/base/file.h @@ -15,8 +15,9 @@ #include -#include "webrtc/base/platform_file.h" #include "webrtc/base/constructormagic.h" +#include "webrtc/base/pathutils.h" +#include "webrtc/base/platform_file.h" namespace rtc { @@ -39,8 +40,12 @@ class File { // Open and Create give files with both reading and writing enabled. static File Open(const std::string& path); + static File Open(Pathname&& path); + static File Open(const Pathname& path); // If the file already exists it will be overwritten. static File Create(const std::string& path); + static File Create(Pathname&& path); + static File Create(const Pathname& path); size_t Write(const uint8_t* data, size_t length); size_t Read(uint8_t* buffer, size_t length); diff --git a/webrtc/base/file_unittest.cc b/webrtc/base/file_unittest.cc index aa787c14c5..d8800a55f4 100644 --- a/webrtc/base/file_unittest.cc +++ b/webrtc/base/file_unittest.cc @@ -163,4 +163,30 @@ TEST_F(FileTest, RandomAccessReadWrite) { EXPECT_TRUE(VerifyBuffer(out, 2, 0)); } +TEST_F(FileTest, OpenFromPathname) { + { + File file = File::Open(Pathname(path_)); + ASSERT_TRUE(file.IsOpen()) << "Error: " << LastError(); + } + + { + Pathname path(path_); + File file = File::Open(path); + ASSERT_TRUE(file.IsOpen()) << "Error: " << LastError(); + } +} + +TEST_F(FileTest, CreateFromPathname) { + { + File file = File::Create(Pathname(path_)); + ASSERT_TRUE(file.IsOpen()) << "Error: " << LastError(); + } + + { + Pathname path(path_); + File file = File::Create(path); + ASSERT_TRUE(file.IsOpen()) << "Error: " << LastError(); + } +} + } // namespace rtc diff --git a/webrtc/base/fileutils.h b/webrtc/base/fileutils.h index 9489de8193..0c19ffc7f7 100644 --- a/webrtc/base/fileutils.h +++ b/webrtc/base/fileutils.h @@ -21,8 +21,7 @@ #include #endif -#include "webrtc/base/basictypes.h" -#include "webrtc/base/common.h" +#include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" #include "webrtc/base/platform_file.h" @@ -175,14 +174,14 @@ class FilesystemInterface { organization_name_ = organization; } void GetOrganizationName(std::string* organization) { - ASSERT(NULL != organization); + RTC_DCHECK(organization); *organization = organization_name_; } void SetApplicationName(const std::string& application) { application_name_ = application; } void GetApplicationName(std::string* application) { - ASSERT(NULL != application); + RTC_DCHECK(application); *application = application_name_; } @@ -194,7 +193,7 @@ class FilesystemInterface { class Filesystem { public: static FilesystemInterface *default_filesystem() { - ASSERT(default_filesystem_ != NULL); + RTC_DCHECK(default_filesystem_); return default_filesystem_; } diff --git a/webrtc/base/flags.h b/webrtc/base/flags.h index 6ca50b5cea..7a7148732e 100644 --- a/webrtc/base/flags.h +++ b/webrtc/base/flags.h @@ -24,7 +24,6 @@ #define WEBRTC_BASE_FLAGS_H__ #include "webrtc/base/checks.h" -#include "webrtc/base/common.h" #include "webrtc/base/constructormagic.h" namespace rtc { diff --git a/webrtc/base/pathutils.cc b/webrtc/base/pathutils.cc index 90869b482a..c4a2c30dc8 100644 --- a/webrtc/base/pathutils.cc +++ b/webrtc/base/pathutils.cc @@ -15,7 +15,7 @@ #include #endif // WEBRTC_WIN -#include "webrtc/base/common.h" +#include "webrtc/base/checks.h" #include "webrtc/base/fileutils.h" #include "webrtc/base/logging.h" #include "webrtc/base/pathutils.h" @@ -72,7 +72,7 @@ Pathname& Pathname::operator=(const Pathname&) = default; Pathname& Pathname::operator=(Pathname&&) = default; void Pathname::SetFolderDelimiter(char delimiter) { - ASSERT(IsFolderDelimiter(delimiter)); + RTC_DCHECK(IsFolderDelimiter(delimiter)); folder_delimiter_ = delimiter; } diff --git a/webrtc/base/pathutils.h b/webrtc/base/pathutils.h index 2a0efa9763..5a5fd1e1fa 100644 --- a/webrtc/base/pathutils.h +++ b/webrtc/base/pathutils.h @@ -12,8 +12,8 @@ #define WEBRTC_BASE_PATHUTILS_H__ #include -// Temporary, until deprecated helpers are removed. -#include "webrtc/base/fileutils.h" + +#include "webrtc/base/checks.h" namespace rtc { @@ -108,63 +108,6 @@ private: char folder_delimiter_; }; -/////////////////////////////////////////////////////////////////////////////// -// Global Helpers (deprecated) -/////////////////////////////////////////////////////////////////////////////// - -inline void SetOrganizationName(const std::string& organization) { - Filesystem::SetOrganizationName(organization); -} -inline void SetApplicationName(const std::string& application) { - Filesystem::SetApplicationName(application); -} -inline void GetOrganizationName(std::string* organization) { - Filesystem::GetOrganizationName(organization); -} -inline void GetApplicationName(std::string* application) { - Filesystem::GetApplicationName(application); -} -inline bool CreateFolder(const Pathname& path) { - return Filesystem::CreateFolder(path); -} -inline bool FinishPath(Pathname& path, bool create, const std::string& append) { - if (!append.empty()) - path.AppendFolder(append); - return !create || CreateFolder(path); -} -// Note: this method uses the convention of / for the temporary -// folder. Filesystem uses /. We will be migrating exclusively -// to // eventually. Since these are temp folders, -// it's probably ok to orphan them during the transition. -inline bool GetTemporaryFolder(Pathname& path, bool create, - const std::string& append) { - std::string application_name; - Filesystem::GetApplicationName(&application_name); - ASSERT(!application_name.empty()); - return Filesystem::GetTemporaryFolder(path, create, &application_name) - && FinishPath(path, create, append); -} -inline bool GetAppDataFolder(Pathname& path, bool create, - const std::string& append) { - ASSERT(!create); // TODO: Support create flag on Filesystem::GetAppDataFolder. - return Filesystem::GetAppDataFolder(&path, true) - && FinishPath(path, create, append); -} -inline bool CleanupTemporaryFolder() { - Pathname path; - if (!GetTemporaryFolder(path, false, "")) - return false; - if (Filesystem::IsAbsent(path)) - return true; - if (!Filesystem::IsTemporaryPath(path)) { - ASSERT(false); - return false; - } - return Filesystem::DeleteFolderContents(path); -} - -/////////////////////////////////////////////////////////////////////////////// - } // namespace rtc #endif // WEBRTC_BASE_PATHUTILS_H__ diff --git a/webrtc/base/urlencode.cc b/webrtc/base/urlencode.cc index 8dc185dfb7..bb508f4621 100644 --- a/webrtc/base/urlencode.cc +++ b/webrtc/base/urlencode.cc @@ -10,7 +10,7 @@ #include "webrtc/base/urlencode.h" -#include "webrtc/base/common.h" +#include "webrtc/base/checks.h" #include "webrtc/base/stringutils.h" static int HexPairValue(const char * code) { @@ -114,7 +114,7 @@ int InternalUrlEncode(const char *source, char *dest, unsigned int max, } source++; } - ASSERT(static_cast(dest - start) < max); + RTC_DCHECK_LT(static_cast(dest - start), max); *dest = 0; return static_cast(dest - start);