From 45895d8d4f7b348b64d0c565271da73a03dab5cc Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 18 Dec 2022 16:34:16 +0100 Subject: [PATCH] We don't actually need to calculate or store the ancestor chain unless we're renaming --- src/cryfs/impl/filesystem/CryDevice.cpp | 27 ++++++++++++------------ src/cryfs/impl/filesystem/CryDevice.h | 6 ++---- src/cryfs/impl/filesystem/CryDir.cpp | 4 ++-- src/cryfs/impl/filesystem/CryDir.h | 2 +- src/cryfs/impl/filesystem/CryFile.cpp | 4 ++-- src/cryfs/impl/filesystem/CryFile.h | 2 +- src/cryfs/impl/filesystem/CryNode.cpp | 10 ++++----- src/cryfs/impl/filesystem/CryNode.h | 3 +-- src/cryfs/impl/filesystem/CrySymlink.cpp | 4 ++-- src/cryfs/impl/filesystem/CrySymlink.h | 2 +- 10 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/cryfs/impl/filesystem/CryDevice.cpp b/src/cryfs/impl/filesystem/CryDevice.cpp index e58c5828..a8dee3d8 100644 --- a/src/cryfs/impl/filesystem/CryDevice.cpp +++ b/src/cryfs/impl/filesystem/CryDevice.cpp @@ -191,17 +191,15 @@ optional> CryDevice::Load(const bf::path &path) { if (path.parent_path().empty()) { //We are asked to load the base directory '/'. - return optional>(make_unique_ref(this, none, none, _rootBlobId, std::vector())); + return optional>(make_unique_ref(this, none, none, _rootBlobId)); } - auto parentWithAncestors = LoadDirBlobWithAncestors(path.parent_path()); + auto parentWithAncestors = LoadDirBlobWithAncestors(path.parent_path(), none); if (parentWithAncestors == none) { return none; } auto parent = std::move(parentWithAncestors->blob); auto grandparent = std::move(parentWithAncestors->parent); - auto ancestors = std::move(parentWithAncestors->ancestors); - ancestors.push_back(parent->blockId()); // parent's ancestors don't contain parent yet, but parent is our ancestor auto optEntry = parent->GetChild(path.filename().string()); if (optEntry == boost::none) { @@ -211,17 +209,17 @@ optional> CryDevice::Load(const bf::path &path) { switch(entry.type()) { case fspp::Dir::EntryType::DIR: - return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId(), std::move(ancestors))); + return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId())); case fspp::Dir::EntryType::FILE: - return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId(), std::move(ancestors))); + return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId())); case fspp::Dir::EntryType::SYMLINK: - return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId(), std::move(ancestors))); + return optional>(make_unique_ref(this, std::move(parent), std::move(grandparent), entry.blockId())); } ASSERT(false, "Switch/case not exhaustive"); } -optional CryDevice::LoadDirBlobWithAncestors(const bf::path &path) { - auto blob = LoadBlobWithAncestors(path); +optional CryDevice::LoadDirBlobWithAncestors(const bf::path &path, boost::optional*> append_ancestors_to) { + auto blob = LoadBlobWithAncestors(path, std::move(append_ancestors_to)); if (blob == none) { return none; } @@ -229,11 +227,10 @@ optional CryDevice::LoadDirBlobWithAncestors(co if (dir == none) { throw FuseErrnoException(ENOTDIR); // Loaded blob is not a directory } - return DirBlobWithAncestors{std::move(*dir), std::move(blob->parent), std::move(blob->ancestors)}; + return DirBlobWithAncestors{std::move(*dir), std::move(blob->parent)}; } -optional CryDevice::LoadBlobWithAncestors(const bf::path &path) { - std::vector ancestors; +optional CryDevice::LoadBlobWithAncestors(const bf::path &path, boost::optional*> append_ancestors_to) { optional> parentBlob = none; optional> currentBlobOpt = _fsBlobStore->load(_rootBlobId); if (currentBlobOpt == none) { @@ -244,7 +241,9 @@ optional CryDevice::LoadBlobWithAncestors(const bf ASSERT(currentBlob->parentPointer() == BlockId::Null(), "Root Blob should have a nullptr as parent"); for (const bf::path &component : path.relative_path()) { - ancestors.push_back(currentBlob->blockId()); + if (append_ancestors_to != boost::none) { + (*append_ancestors_to)->push_back(currentBlob->blockId()); + } auto currentDir = dynamic_pointer_move(currentBlob); if (currentDir == none) { throw FuseErrnoException(ENOTDIR); // Path component is not a dir @@ -265,7 +264,7 @@ optional CryDevice::LoadBlobWithAncestors(const bf ASSERT(currentBlob->parentPointer() == (*parentBlob)->blockId(), "Blob has wrong parent pointer"); } - return BlobWithAncestors{std::move(currentBlob), std::move(parentBlob), std::move(ancestors)}; + return BlobWithAncestors{std::move(currentBlob), std::move(parentBlob)}; //TODO (I think this is resolved, but I should test it) // Running the python script, waiting for "Create files in sequential order...", then going into dir ~/tmp/cryfs-mount-.../Bonnie.../ and calling "ls" diff --git a/src/cryfs/impl/filesystem/CryDevice.h b/src/cryfs/impl/filesystem/CryDevice.h index 734d7d4c..53dd2e44 100644 --- a/src/cryfs/impl/filesystem/CryDevice.h +++ b/src/cryfs/impl/filesystem/CryDevice.h @@ -31,9 +31,8 @@ public: struct DirBlobWithAncestors { cpputils::unique_ref blob; boost::optional> parent; - std::vector ancestors; }; - boost::optional LoadDirBlobWithAncestors(const boost::filesystem::path &path); + boost::optional LoadDirBlobWithAncestors(const boost::filesystem::path &path, boost::optional*> append_ancestors_to); void RemoveBlob(const blockstore::BlockId &blockId); void onFsAction(std::function callback); @@ -69,9 +68,8 @@ private: struct BlobWithAncestors { cpputils::unique_ref blob; boost::optional> parent; - std::vector ancestors; }; - boost::optional LoadBlobWithAncestors(const boost::filesystem::path &path); + boost::optional LoadBlobWithAncestors(const boost::filesystem::path &path, boost::optional*> append_ancestors_to); DISALLOW_COPY_AND_ASSIGN(CryDevice); }; diff --git a/src/cryfs/impl/filesystem/CryDir.cpp b/src/cryfs/impl/filesystem/CryDir.cpp index 5150a340..9dff77f2 100644 --- a/src/cryfs/impl/filesystem/CryDir.cpp +++ b/src/cryfs/impl/filesystem/CryDir.cpp @@ -28,8 +28,8 @@ using cryfs::parallelaccessfsblobstore::DirBlobRef; namespace cryfs { -CryDir::CryDir(CryDevice *device, optional> parent, optional> grandparent, const BlockId &blockId, std::vector ancestors) -: CryNode(device, std::move(parent), std::move(grandparent), blockId, std::move(ancestors)) { +CryDir::CryDir(CryDevice *device, optional> parent, optional> grandparent, const BlockId &blockId) +: CryNode(device, std::move(parent), std::move(grandparent), blockId) { } CryDir::~CryDir() { diff --git a/src/cryfs/impl/filesystem/CryDir.h b/src/cryfs/impl/filesystem/CryDir.h index 5e4b15ce..74343774 100644 --- a/src/cryfs/impl/filesystem/CryDir.h +++ b/src/cryfs/impl/filesystem/CryDir.h @@ -10,7 +10,7 @@ namespace cryfs { class CryDir final: public fspp::Dir, public CryNode { public: - CryDir(CryDevice *device, boost::optional> parent, boost::optional> grandparent, const blockstore::BlockId &blockId, std::vector ancestors); + CryDir(CryDevice *device, boost::optional> parent, boost::optional> grandparent, const blockstore::BlockId &blockId); ~CryDir(); //TODO return type variance to CryFile/CryDir? diff --git a/src/cryfs/impl/filesystem/CryFile.cpp b/src/cryfs/impl/filesystem/CryFile.cpp index 9a83c553..b598b04c 100644 --- a/src/cryfs/impl/filesystem/CryFile.cpp +++ b/src/cryfs/impl/filesystem/CryFile.cpp @@ -18,8 +18,8 @@ using cryfs::parallelaccessfsblobstore::FileBlobRef; namespace cryfs { -CryFile::CryFile(CryDevice *device, unique_ref parent, optional> grandparent, const BlockId &blockId, std::vector ancestors) -: CryNode(device, std::move(parent), std::move(grandparent), blockId, std::move(ancestors)) { +CryFile::CryFile(CryDevice *device, unique_ref parent, optional> grandparent, const BlockId &blockId) +: CryNode(device, std::move(parent), std::move(grandparent), blockId) { } CryFile::~CryFile() { diff --git a/src/cryfs/impl/filesystem/CryFile.h b/src/cryfs/impl/filesystem/CryFile.h index 1e429332..4e66e1ca 100644 --- a/src/cryfs/impl/filesystem/CryFile.h +++ b/src/cryfs/impl/filesystem/CryFile.h @@ -11,7 +11,7 @@ namespace cryfs { class CryFile final: public fspp::File, public CryNode { public: - CryFile(CryDevice *device, cpputils::unique_ref parent, boost::optional> grandparent, const blockstore::BlockId &blockId, std::vector ancestors); + CryFile(CryDevice *device, cpputils::unique_ref parent, boost::optional> grandparent, const blockstore::BlockId &blockId); ~CryFile(); cpputils::unique_ref open(fspp::openflags_t flags) override; diff --git a/src/cryfs/impl/filesystem/CryNode.cpp b/src/cryfs/impl/filesystem/CryNode.cpp index 0ad7c1ba..59ba2502 100644 --- a/src/cryfs/impl/filesystem/CryNode.cpp +++ b/src/cryfs/impl/filesystem/CryNode.cpp @@ -18,6 +18,7 @@ using cpputils::dynamic_pointer_move; using boost::optional; using boost::none; using std::shared_ptr; +using std::vector; using cryfs::parallelaccessfsblobstore::FsBlobRef; using cryfs::parallelaccessfsblobstore::DirBlobRef; using namespace cpputils::logging; @@ -27,11 +28,10 @@ using fspp::fuse::FuseErrnoException; namespace cryfs { -CryNode::CryNode(CryDevice *device, optional> parent, optional> grandparent, const BlockId &blockId, std::vector ancestors) +CryNode::CryNode(CryDevice *device, optional> parent, optional> grandparent, const BlockId &blockId) : _device(device), _parent(none), _grandparent(none), - _ancestors(std::move(ancestors)), _blockId(blockId) { ASSERT(parent != none || grandparent == none, "Grandparent can only be set when parent is not none"); @@ -88,15 +88,15 @@ void CryNode::rename(const bf::path &to) { throw FuseErrnoException(EBUSY); } - auto targetParentAndAncestors = _device->LoadDirBlobWithAncestors(to.parent_path()); + vector targetAncestors; + auto targetParentAndAncestors = _device->LoadDirBlobWithAncestors(to.parent_path(), &targetAncestors); if (targetParentAndAncestors == none) { // Target parent directory doesn't exist throw FuseErrnoException(ENOENT); } auto targetParent = std::move(targetParentAndAncestors->blob); auto targetGrandparent = std::move(targetParentAndAncestors->parent); - auto targetAncestors = std::move(targetParentAndAncestors->ancestors); - targetAncestors.push_back(targetParent->blockId()); // targetParent is not an ancestor of the targetParent, but it's a target ancestor + targetAncestors.push_back(targetParent->blockId()); // targetParent is not an ancestor of the targetParent, so it wasn't filled in, but it's a target ancestor if (std::find(targetAncestors.begin(), targetAncestors.end(), _blockId) != targetAncestors.end()) { // We are trying to move a node into one of its subdirectories. This is not allowed. diff --git a/src/cryfs/impl/filesystem/CryNode.h b/src/cryfs/impl/filesystem/CryNode.h index a3e31443..c613ebb9 100644 --- a/src/cryfs/impl/filesystem/CryNode.h +++ b/src/cryfs/impl/filesystem/CryNode.h @@ -15,7 +15,7 @@ public: virtual ~CryNode(); // TODO grandparent is only needed to set the timestamps of the parent directory on rename and remove. Delete grandparent parameter once we store timestamps in the blob itself instead of in the directory listing. - CryNode(CryDevice *device, boost::optional> parent, boost::optional> grandparent, const blockstore::BlockId &blockId, std::vector ancestors); + CryNode(CryDevice *device, boost::optional> parent, boost::optional> grandparent, const blockstore::BlockId &blockId); void access(int mask) const override; stat_info stat() const override; @@ -51,7 +51,6 @@ private: CryDevice *_device; boost::optional> _parent; boost::optional> _grandparent; - std::vector _ancestors; // [n-1] is the parent, [n-2] is the grandparent, ..., [0] is the file system root, blockstore::BlockId _blockId; DISALLOW_COPY_AND_ASSIGN(CryNode); diff --git a/src/cryfs/impl/filesystem/CrySymlink.cpp b/src/cryfs/impl/filesystem/CrySymlink.cpp index 3e21836e..5b99ba79 100644 --- a/src/cryfs/impl/filesystem/CrySymlink.cpp +++ b/src/cryfs/impl/filesystem/CrySymlink.cpp @@ -21,8 +21,8 @@ using cryfs::parallelaccessfsblobstore::DirBlobRef; namespace cryfs { -CrySymlink::CrySymlink(CryDevice *device, unique_ref parent, optional> grandparent, const BlockId &blockId, std::vector ancestors) -: CryNode(device, std::move(parent), std::move(grandparent), blockId, std::move(ancestors)) { +CrySymlink::CrySymlink(CryDevice *device, unique_ref parent, optional> grandparent, const BlockId &blockId) +: CryNode(device, std::move(parent), std::move(grandparent), blockId) { } CrySymlink::~CrySymlink() { diff --git a/src/cryfs/impl/filesystem/CrySymlink.h b/src/cryfs/impl/filesystem/CrySymlink.h index a906a9b4..ed8acea5 100644 --- a/src/cryfs/impl/filesystem/CrySymlink.h +++ b/src/cryfs/impl/filesystem/CrySymlink.h @@ -11,7 +11,7 @@ namespace cryfs { class CrySymlink final: public fspp::Symlink, public CryNode { public: - CrySymlink(CryDevice *device, cpputils::unique_ref parent, boost::optional> grandparent, const blockstore::BlockId &blockId, std::vector ancestors); + CrySymlink(CryDevice *device, cpputils::unique_ref parent, boost::optional> grandparent, const blockstore::BlockId &blockId); ~CrySymlink(); boost::filesystem::path target() override;