From 448922ef986eb3d99b724ad490b91e98c906b13d Mon Sep 17 00:00:00 2001 From: sbc Date: Wed, 7 Jan 2015 04:28:25 -0800 Subject: [NaCl SDK] nacl_io: Remove unused methods from Path class and update Path.Append Previously Path.Append would strip the leading / when appending absolute paths. This change make the behaviour more like python's os.path.join() such that if you try to append an absolute path is clobbers the existing one. This fits with the behaviour we more often require especially for the upcoming additions of symlink code. Review URL: https://codereview.chromium.org/832173003 Cr-Commit-Position: refs/heads/master@{#310269} --- native_client_sdk/src/libraries/nacl_io/dir_node.h | 2 +- .../src/libraries/nacl_io/html5fs/html5_fs.cc | 9 ++++- .../src/libraries/nacl_io/httpfs/http_fs.cc | 3 +- .../src/libraries/nacl_io/kernel_object.cc | 15 ++------ .../src/libraries/nacl_io/memfs/mem_fs.cc | 8 ---- native_client_sdk/src/libraries/nacl_io/path.cc | 45 ++++++++++------------ native_client_sdk/src/libraries/nacl_io/path.h | 4 +- .../src/tests/nacl_io_test/kernel_proxy_test.cc | 1 + .../src/tests/nacl_io_test/path_test.cc | 22 ++++------- 9 files changed, 44 insertions(+), 65 deletions(-) (limited to 'native_client_sdk') diff --git a/native_client_sdk/src/libraries/nacl_io/dir_node.h b/native_client_sdk/src/libraries/nacl_io/dir_node.h index d8b5767..c188d3f 100644 --- a/native_client_sdk/src/libraries/nacl_io/dir_node.h +++ b/native_client_sdk/src/libraries/nacl_io/dir_node.h @@ -24,7 +24,7 @@ typedef sdk_util::ScopedRef ScopedDirNode; class DirNode : public Node { protected: - explicit DirNode(Filesystem* fs, mode_t mode); + DirNode(Filesystem* fs, mode_t mode); virtual ~DirNode(); public: diff --git a/native_client_sdk/src/libraries/nacl_io/html5fs/html5_fs.cc b/native_client_sdk/src/libraries/nacl_io/html5fs/html5_fs.cc index 9a5aaf3..055a88a 100644 --- a/native_client_sdk/src/libraries/nacl_io/html5fs/html5_fs.cc +++ b/native_client_sdk/src/libraries/nacl_io/html5fs/html5_fs.cc @@ -90,8 +90,13 @@ Error Html5Fs::OpenWithMode(const Path& path, int open_flags, mode_t mode, } Path Html5Fs::GetFullPath(const Path& path) { - Path full_path(path); - full_path.Prepend(prefix_); + if (prefix_.empty()) + return path; + + Path rel_path(path); + rel_path.MakeRelative(); + Path full_path(prefix_); + full_path.Append(rel_path); return full_path; } diff --git a/native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc b/native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc index 43ec0cf..3a76f8c 100644 --- a/native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc +++ b/native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc @@ -405,8 +405,7 @@ Error HttpFs::LoadManifest(const std::string& manifest_name, } std::string HttpFs::MakeUrl(const Path& path) { - return url_root_ + - (path.IsAbsolute() ? path.Range(1, path.Size()) : path.Join()); + return url_root_ + Path(path).MakeRelative().Join(); } } // namespace nacl_io diff --git a/native_client_sdk/src/libraries/nacl_io/kernel_object.cc b/native_client_sdk/src/libraries/nacl_io/kernel_object.cc index 18c9f93..1ec040a 100644 --- a/native_client_sdk/src/libraries/nacl_io/kernel_object.cc +++ b/native_client_sdk/src/libraries/nacl_io/kernel_object.cc @@ -86,7 +86,7 @@ Error KernelObject::AcquireFsAndRelPath(const std::string& path, FsMap_t::iterator it = filesystems_.find(abs_parts.Range(0, max - len)); if (it != filesystems_.end()) { rel_parts->Set("/"); - rel_parts->Append(abs_parts.Range(max - len, max)); + rel_parts->Append(Path(abs_parts.Range(max - len, max))); *out_fs = it->second; return 0; @@ -118,22 +118,13 @@ Error KernelObject::AcquireFsAndNode(const std::string& path, Path KernelObject::GetAbsParts(const std::string& path) { AUTO_LOCK(cwd_lock_); - - Path abs_parts; - if (path[0] == '/') { - abs_parts = path; - } else { - abs_parts = cwd_; - abs_parts.Append(path); - } - - return abs_parts; + Path abs_parts(cwd_); + return abs_parts.Append(Path(path)); } std::string KernelObject::GetCWD() { AUTO_LOCK(cwd_lock_); std::string out = cwd_; - return out; } diff --git a/native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc b/native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc index 70da25c..c1ca8ed 100644 --- a/native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc +++ b/native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc @@ -118,10 +118,6 @@ Error MemFs::OpenWithMode(const Path& path, int open_flags, mode_t mode, } Error MemFs::Mkdir(const Path& path, int mode) { - // We expect a Filesystem "absolute" path - if (!path.IsAbsolute()) - return ENOENT; - // The root of the filesystem is already created by the filesystem if (path.Size() == 1) return EEXIST; @@ -237,10 +233,6 @@ Error MemFs::RemoveInternal(const Path& path, int remove_type) { bool remove_dir = (remove_type & REMOVE_DIR) != 0; if (dir_only) { - // We expect a Filesystem "absolute" path - if (!path.IsAbsolute()) - return ENOENT; - // The root of the filesystem is already created by the filesystem if (path.Size() == 1) return EEXIST; diff --git a/native_client_sdk/src/libraries/nacl_io/path.cc b/native_client_sdk/src/libraries/nacl_io/path.cc index 10c93a9..c524bfa 100644 --- a/native_client_sdk/src/libraries/nacl_io/path.cc +++ b/native_client_sdk/src/libraries/nacl_io/path.cc @@ -36,42 +36,39 @@ bool Path::IsRoot() const { return paths_.empty() || (paths_.size() == 1 && paths_[0] == "/"); } -Path& Path::Append(const std::string& path) { - StringArray_t paths = Split(path); - if (paths.empty()) - return *this; +Path& Path::MakeRelative() { + if (IsAbsolute()) { + paths_.erase(paths_.begin()); + } + return *this; +} - for (size_t index = 0; index < paths.size(); index++) { - // Skip ROOT - if (paths_.size() && index == 0 && paths[0] == "/") - continue; - paths_.push_back(paths[index]); +Path& Path::Append(const Path& path) { + // Appending an absolute path effectivly sets the path, ignoring + // the current contents. + if (path.IsAbsolute()) { + paths_ = path.paths_; + } else { + for (size_t index = 0; index < path.paths_.size(); index++) { + paths_.push_back(path.paths_[index]); + } } paths_ = Normalize(paths_); return *this; } -Path& Path::Prepend(const std::string& path) { - StringArray_t paths = Split(path); - if (paths.empty()) - return *this; - - for (size_t index = 0; index < paths_.size(); index++) { - // Skip ROOT - if (index == 0 && paths_[0] == "/") - continue; - paths.push_back(paths_[index]); - } +Path& Path::Append(const std::string& path) { + return Append(Path(path)); +} - paths_ = Normalize(paths); +Path& Path::Set(const StringArray_t path) { + paths_ = Normalize(path); return *this; } Path& Path::Set(const std::string& path) { - StringArray_t paths = Split(path); - paths_ = Normalize(paths); - return *this; + return Set(Split(path)); } Path Path::Parent() const { diff --git a/native_client_sdk/src/libraries/nacl_io/path.h b/native_client_sdk/src/libraries/nacl_io/path.h index c2a7c5e..ff90113 100644 --- a/native_client_sdk/src/libraries/nacl_io/path.h +++ b/native_client_sdk/src/libraries/nacl_io/path.h @@ -37,9 +37,11 @@ class Path { size_t Size() const; // Update the path. + Path& Append(const Path& path); Path& Append(const std::string& path); - Path& Prepend(const std::string& path); Path& Set(const std::string& path); + Path& MakeRelative(); + Path& Set(const StringArray_t path); // Return the parent path. Path Parent() const; diff --git a/native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc b/native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc index 62f8c6f..c8ce993 100644 --- a/native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc +++ b/native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc @@ -1045,6 +1045,7 @@ TEST_F(KernelProxyErrorTest, WriteError) { .WillOnce(DoAll(SetArgPointee<3>(0), // Wrote 0 bytes. Return(1234))); // Returned error 1234. + EXPECT_CALL(*mock_node, IsaDir()).Times(1); EXPECT_CALL(*mock_node, Destroy()).Times(1); int fd = ki_open("/dummy", O_WRONLY, 0); diff --git a/native_client_sdk/src/tests/nacl_io_test/path_test.cc b/native_client_sdk/src/tests/nacl_io_test/path_test.cc index 12aa2d9..9cdaa682 100644 --- a/native_client_sdk/src/tests/nacl_io_test/path_test.cc +++ b/native_client_sdk/src/tests/nacl_io_test/path_test.cc @@ -189,17 +189,21 @@ TEST(PathTest, AppendAndJoin) { EXPECT_EQ("/", ph3.Join()); ph3 = ph3.Append("USR/local/SHARE"); EXPECT_EQ("/USR/local/SHARE", ph3.Join()); - ph3 = ph3.Append("///////////////////////////////"); - EXPECT_EQ("/USR/local/SHARE", ph3.Join()); Path ph4(".."); EXPECT_EQ("..", ph4.Join()); - ph4 = ph4.Append("/node1/node3/../../node1/./"); + ph4 = ph4.Append("node1/node3/../../node1/./"); EXPECT_EQ("../node1", ph4.Join()); ph4 = ph4.Append("node4/../../node1/./node5"); EXPECT_EQ("../node1/node5", ph4.Join()); } +TEST(PathTest, AppendAbsolute) { + Path path("/usr/local"); + path.Append("/foo"); + EXPECT_EQ("/foo", path.Join()); +} + TEST(PathTest, Invalid) { Path absolute("/usr/local"); Path current("./usr/local"); @@ -257,15 +261,3 @@ TEST(PathTest, Range) { EXPECT_EQ("an/absolute", p.Range(1, 3)); EXPECT_EQ("absolute", p.Range(2, 3)); } - -TEST(PathTest, PrependRelative) { - Path p("foo/bar"); - p.Prepend("prefix"); - EXPECT_EQ("prefix/foo/bar", p.Join()); -} - -TEST(PathTest, PrependAbsolute) { - Path p("/foo/bar"); - p.Prepend("/prefix"); - EXPECT_EQ("/prefix/foo/bar", p.Join()); -} -- cgit v1.1