From 97de8ddbf63410271249413a8cfc3988adcf6c36 Mon Sep 17 00:00:00 2001 From: bergmann Date: Fri, 6 Dec 2019 00:24:35 +0100 Subject: [PATCH] * Implemented comparison of directory entries and path * Fixed bug: Copy directory iterator caused SEGFAULT and wrong listings --- include/cppfs/directory/directory.h | 26 +++++-- include/cppfs/directory/directory.inl | 78 ++++++++++++++++++++- include/cppfs/directory/directory.linux.inl | 45 +++++++----- include/cppfs/path/path.h | 14 ++++ include/cppfs/path/path.inl | 36 ++++++++++ test/cppfs/directory_tests.cpp | 72 ++++++++++++++++++- 6 files changed, 247 insertions(+), 24 deletions(-) diff --git a/include/cppfs/directory/directory.h b/include/cppfs/directory/directory.h index 0b4cc5d..5fa29da 100644 --- a/include/cppfs/directory/directory.h +++ b/include/cppfs/directory/directory.h @@ -21,20 +21,33 @@ namespace cppfs { public: file_type type; - std::string name; + cppfs::path path; + + public: + static inline int compare(const entry& lhv, const entry& rhv); + + public: + inline friend bool operator==(const entry& lhv, const entry& rhv); + inline friend bool operator!=(const entry& lhv, const entry& rhv); + inline friend bool operator<=(const entry& lhv, const entry& rhv); + inline friend bool operator>=(const entry& lhv, const entry& rhv); + inline friend bool operator< (const entry& lhv, const entry& rhv); + inline friend bool operator> (const entry& lhv, const entry& rhv); }; struct iterator : public std::iterator { private: - const directory * _owner; - entry _entry; + cppfs::path _path; + entry _entry; public: + inline iterator() = default; + inline iterator( const directory& p_owner, - bool p_end); + bool p_end); inline iterator( iterator&& p_other); @@ -58,8 +71,9 @@ namespace cppfs private: using dir_ptr_u = std::unique_ptr; - dir_ptr_u _dir; - struct dirent * _dirent; + dir_ptr_u _dir { nullptr, &closedir }; + long _pos { 0 }; + struct dirent * _dirent { nullptr }; inline void next(); inline long offset() const; diff --git a/include/cppfs/directory/directory.inl b/include/cppfs/directory/directory.inl index c4fbf18..31bcc47 100644 --- a/include/cppfs/directory/directory.inl +++ b/include/cppfs/directory/directory.inl @@ -1,5 +1,7 @@ #pragma once +#include + #include "directory.h" #include "../misc.h" #include "../file.h" @@ -7,6 +9,80 @@ namespace cppfs { + /* directory::entry */ + + int directory::entry::compare(const entry& lhv, const entry& rhv) + { + cppcore::op_invariant_string_compare str_cmp; + + auto lhp = lhv.path.parts(); + auto rhp = rhv.path.parts(); + + auto lhi = lhp.begin(); + auto rhi = rhp.begin(); + + auto lhc = lhp.size(); + auto rhc = rhp.size(); + + auto lhd = lhv.type == file_type::directory ? 0 : 1; + auto rhd = rhv.type == file_type::directory ? 0 : 1; + + /* check parts of the path (except the last one) */ + + while (lhc > 0 && rhc > 0) + { + auto lhx = lhc == 1 ? lhd : 0; + auto rhx = rhc == 1 ? rhd : 0; + + /* check type (directory or file) */ + + if (lhx < rhx) + return -1; + if (lhx > rhx) + return 1; + + /* check name */ + + auto cmp = str_cmp(*lhi, *rhi); + if (cmp != 0) + return cmp; + + ++lhi; + ++rhi; + + --lhc; + --rhc; + } + + /* check length of the path */ + + if (lhc < rhc) + return -1; + if (lhc > rhc) + return 1; + + return 0; + } + + bool operator==(const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) == 0; } + + bool operator!=(const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) != 0; } + + bool operator<=(const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) <= 0; } + + bool operator>=(const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) >= 0; } + + bool operator< (const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) < 0; } + + bool operator> (const directory::entry& lhv, const directory::entry& rhv) + { return directory::entry::compare(lhv, rhv) > 0; } + + /* directory */ template @@ -27,7 +103,7 @@ namespace cppfs { for (auto& e : *this) { - auto p = path().join(e.name); + auto p = path().join(e.path); switch (e.type) { case file_type::directory: diff --git a/include/cppfs/directory/directory.linux.inl b/include/cppfs/directory/directory.linux.inl index 3d05a76..33f895d 100644 --- a/include/cppfs/directory/directory.linux.inl +++ b/include/cppfs/directory/directory.linux.inl @@ -10,15 +10,15 @@ namespace cppfs /* directory::iterator */ directory::iterator::iterator(const directory& p_owner, bool p_end) - : _owner (&p_owner) + : _path (p_owner.path()) , _entry () , _dir (nullptr, &closedir) + , _pos (-1) , _dirent (nullptr) { if (!p_end) { - assert(_owner); - auto s = _owner->path().str(); + auto s = _path.str(); _dir.reset(opendir(s.c_str())); if (!_dir) throw cppcore::error_exception("Unable to open directory", errno); @@ -27,44 +27,58 @@ namespace cppfs } directory::iterator::iterator(iterator&& p_other) - : _owner (std::move(p_other)._owner) + : _path (std::move(p_other)._path) , _entry (std::move(p_other)._entry) , _dir (std::move(p_other)._dir) + , _pos (std::move(p_other)._pos) , _dirent (std::move(p_other)._dirent) - { } + { + p_other._dir.reset(); + p_other._pos = -1; + p_other._dirent = nullptr; + } directory::iterator::iterator(const iterator& p_other) - : _owner (p_other._owner) + : _path (p_other._path) , _entry () , _dir (nullptr, &closedir) + , _pos (-1) , _dirent (nullptr) { this->operator=(p_other); } directory::iterator& directory::iterator::operator=(iterator&& p_other) { - _owner = std::move(p_other)._owner; + _path = std::move(p_other)._path; _entry = std::move(p_other)._entry; _dir = std::move(p_other)._dir; + _pos = std::move(p_other)._pos; _dirent = std::move(p_other)._dirent; + p_other._dir.reset(); + p_other._pos = -1; + p_other._dirent = nullptr; + return *this; } directory::iterator& directory::iterator::operator=(const iterator& p_other) { - _owner = p_other._owner; + _path = p_other._path; + _pos = p_other._pos; _entry = entry { }; _dirent = nullptr; _dir.reset(); - assert(_owner); - auto s = _owner->path().str(); + auto s = _path.str(); _dir.reset(opendir(s.c_str())); if (!_dir) throw cppcore::error_exception("Unable to open directory", errno); - seekdir(_dir.get(), p_other.offset()); - next(); + if (_dir && _pos >= 0) + { + seekdir(_dir.get(), _pos); + next(); + } return *this; } @@ -84,9 +98,7 @@ namespace cppfs bool directory::iterator::operator==(const iterator& p_other) const { - return _owner - && p_other._owner - && _owner == p_other._owner + return _path == p_other._path && ( (!_dirent && !p_other._dirent) || offset() == p_other.offset()); } @@ -110,6 +122,7 @@ namespace cppfs { do { + _pos = offset(); _dirent = readdir(_dir.get()); } while ( @@ -122,7 +135,7 @@ namespace cppfs if (_dirent) { - _entry.name = &_dirent->d_name[0]; + _entry.path = _path.join(std::string(&_dirent->d_name[0])); switch (_dirent->d_type) { case DT_BLK: _entry.type = file_type::block_device; break; diff --git a/include/cppfs/path/path.h b/include/cppfs/path/path.h index 6819ca0..9f5c627 100644 --- a/include/cppfs/path/path.h +++ b/include/cppfs/path/path.h @@ -139,6 +139,20 @@ namespace cppfs */ inline path join(const path& other) const; + public: + inline friend bool operator< (const path& lhv, const path& rhv); + inline friend bool operator<=(const path& lhv, const path& rhv); + inline friend bool operator==(const path& lhv, const path& rhv); + inline friend bool operator!=(const path& lhv, const path& rhv); + inline friend bool operator>=(const path& lhv, const path& rhv); + inline friend bool operator> (const path& lhv, const path& rhv); + + public: + /** + * @brief Compare two paths. + */ + static inline int compare(const path& lhv, const path& rhv); + private: /** * @brief Create a path from the parts vector. diff --git a/include/cppfs/path/path.inl b/include/cppfs/path/path.inl index d96adfd..88a61d1 100644 --- a/include/cppfs/path/path.inl +++ b/include/cppfs/path/path.inl @@ -119,4 +119,40 @@ namespace cppfs } } + bool operator< (const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) < 0; } + + bool operator<=(const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) <= 0; } + + bool operator==(const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) == 0; } + + bool operator!=(const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) != 0; } + + bool operator>=(const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) >= 0; } + + bool operator> (const path& lhv, const path& rhv) + { return path::compare(lhv, rhv) > 0; } + + + int path::compare(const path& lhv, const path& rhv) + { + auto& lhp = lhv.parts(); + auto& rhp = rhv.parts(); + + auto lht = lhv._status == path_status::absoulte ? 0 : 1; + auto rht = rhv._status == path_status::absoulte ? 0 : 1; + + return + lht < rht ? -1 : + lht > rht ? 1 : + lhp < rhp ? -1 : + rhp < rhp ? 1 : + 0; + + } + } diff --git a/test/cppfs/directory_tests.cpp b/test/cppfs/directory_tests.cpp index 13a6408..c64fd73 100644 --- a/test/cppfs/directory_tests.cpp +++ b/test/cppfs/directory_tests.cpp @@ -24,7 +24,7 @@ TEST(directory_tests, iterator) directory dir("./cppfs"); std::vector files; - std::transform(dir.begin(), dir.end(), std::back_inserter(files), [](auto& e) { return e.name; }); + std::transform(dir.begin(), dir.end(), std::back_inserter(files), [](auto& e) { return e.path.parts().back(); }); std::sort(files.begin(), files.end()); EXPECT_EQ( @@ -35,3 +35,73 @@ TEST(directory_tests, iterator) "path_tests.cpp", })); } + +TEST(directory_tests, entry_compare) +{ + using namespace std::string_literals; + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::directory, "/some/test/aaa"s }, + directory::entry { file_type::directory, "/Some/Test/ZzZ"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::directory, "/some/test"s }, + directory::entry { file_type::directory, "/Some/Test/ZzZ"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::directory, "/some/test/ZZZ"s }, + directory::entry { file_type::file, "/Some/Test/AAA"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::file, "/Some/aaa/AAA"s }, + directory::entry { file_type::directory, "/some/bbb"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::file, "/some/test/aAa"s }, + directory::entry { file_type::file, "/Some/Test/ZzZ"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::file, "/Some/Test/ZzZ"s }, + directory::entry { file_type::file, "/some/test"s })); + + EXPECT_EQ(-1, directory::entry::compare( + directory::entry { file_type::file, "/folder/file"s }, + directory::entry { file_type::file, "/file"s })); + + + + EXPECT_EQ(0, directory::entry::compare( + directory::entry { file_type::file, "/some/test/path"s }, + directory::entry { file_type::file, "/Some/Test/paTh"s })); + + + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::directory, "/Some/Test/ZzZ"s }, + directory::entry { file_type::directory, "/some/test/aaa"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::directory, "/Some/Test/ZzZ"s }, + directory::entry { file_type::directory, "/some/test"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::file, "/Some/Test/AAA"s }, + directory::entry { file_type::directory, "/some/test/ZZZ"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::directory, "/some/bbb"s }, + directory::entry { file_type::file, "/Some/aaa/AAA"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::file, "/Some/Test/ZzZ"s }, + directory::entry { file_type::file, "/some/test/aAa"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::file, "/some/test"s }, + directory::entry { file_type::file, "/Some/Test/ZzZ"s })); + + EXPECT_EQ(1, directory::entry::compare( + directory::entry { file_type::file, "/file"s }, + directory::entry { file_type::file, "/folder/file"s })); + +}