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}
diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn
index 2dea258..10ad64e 100644
--- a/webrtc/base/BUILD.gn
+++ b/webrtc/base/BUILD.gn
@@ -126,6 +126,8 @@
"event_tracer.h",
"file.cc",
"file.h",
+ "flags.cc",
+ "flags.h",
"format_macros.h",
"function_view.h",
"ignore_wundef.h",
@@ -139,6 +141,8 @@
"onetimeevent.h",
"optional.cc",
"optional.h",
+ "pathutils.cc",
+ "pathutils.h",
"platform_file.cc",
"platform_file.h",
"platform_thread.cc",
@@ -179,6 +183,8 @@
"timeutils.h",
"trace_event.h",
"type_traits.h",
+ "urlencode.cc",
+ "urlencode.h",
]
if (is_android) {
@@ -413,8 +419,6 @@
"filerotatingstream.h",
"fileutils.cc",
"fileutils.h",
- "flags.cc",
- "flags.h",
"gunit_prod.h",
"helpers.cc",
"helpers.h",
@@ -451,8 +455,6 @@
"opensslidentity.h",
"opensslstreamadapter.cc",
"opensslstreamadapter.h",
- "pathutils.cc",
- "pathutils.h",
"physicalsocketserver.cc",
"physicalsocketserver.h",
"proxydetect.cc",
@@ -509,8 +511,6 @@
"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 74d4817..8b5df9e 100644
--- a/webrtc/base/file.cc
+++ b/webrtc/base/file.cc
@@ -10,8 +10,19 @@
#include "webrtc/base/file.h"
+#include <utility>
+
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 @@
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 10fb698..5e8449c 100644
--- a/webrtc/base/file.h
+++ b/webrtc/base/file.h
@@ -15,8 +15,9 @@
#include <string>
-#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 @@
// 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 aa787c1..d8800a5 100644
--- a/webrtc/base/file_unittest.cc
+++ b/webrtc/base/file_unittest.cc
@@ -163,4 +163,30 @@
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 9489de8..0c19ffc 100644
--- a/webrtc/base/fileutils.h
+++ b/webrtc/base/fileutils.h
@@ -21,8 +21,7 @@
#include <unistd.h>
#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 @@
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 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 6ca50b5..7a71487 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 90869b4..c4a2c30 100644
--- a/webrtc/base/pathutils.cc
+++ b/webrtc/base/pathutils.cc
@@ -15,7 +15,7 @@
#include <tchar.h>
#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=(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 2a0efa9..5a5fd1e 100644
--- a/webrtc/base/pathutils.h
+++ b/webrtc/base/pathutils.h
@@ -12,8 +12,8 @@
#define WEBRTC_BASE_PATHUTILS_H__
#include <string>
-// Temporary, until deprecated helpers are removed.
-#include "webrtc/base/fileutils.h"
+
+#include "webrtc/base/checks.h"
namespace rtc {
@@ -108,63 +108,6 @@
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 <temp>/<appname> for the temporary
-// folder. Filesystem uses <temp>/<exename>. We will be migrating exclusively
-// to <temp>/<orgname>/<appname> 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 8dc185d..bb508f4 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 @@
}
source++;
}
- ASSERT(static_cast<unsigned int>(dest - start) < max);
+ RTC_DCHECK_LT(static_cast<unsigned int>(dest - start), max);
*dest = 0;
return static_cast<int>(dest - start);
