From bf0777fd9e62852ecda34bc31b2f10b8de1c2cb8 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 27 Sep 2016 23:13:59 +0200 Subject: [PATCH] fixed update of DirHash list and FileHash list when files and dirs are removed, while keeping the cost low --- .../src/file_sharing/dir_hierarchy.cc | 88 +++++++++++++------ .../src/file_sharing/dir_hierarchy.h | 6 +- 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/libretroshare/src/file_sharing/dir_hierarchy.cc b/libretroshare/src/file_sharing/dir_hierarchy.cc index 3c360d973..3b0e49bed 100644 --- a/libretroshare/src/file_sharing/dir_hierarchy.cc +++ b/libretroshare/src/file_sharing/dir_hierarchy.cc @@ -61,8 +61,6 @@ InternalFileHierarchyStorage::InternalFileHierarchyStorage() : mRoot(0) de->dir_hash=RsFileHash() ; // null hash is root by convention. mNodes.push_back(de) ; - -#warning not very elegant. We should remove the leading / mDirHashes[de->dir_hash] = 0 ; } @@ -75,24 +73,43 @@ bool InternalFileHierarchyStorage::getDirHashFromIndex(const DirectoryStorage::E return true; } -bool InternalFileHierarchyStorage::getIndexFromDirHash(const RsFileHash& hash,DirectoryStorage::EntryIndex& index) const +bool InternalFileHierarchyStorage::getIndexFromDirHash(const RsFileHash& hash,DirectoryStorage::EntryIndex& index) { - std::map::const_iterator it = mDirHashes.find(hash) ; + std::map::iterator it = mDirHashes.find(hash) ; if(it == mDirHashes.end()) return false; index = it->second; + + // make sure the hash actually points to some existing file. If not, remove it. This is a lazy update of dir hashes: when we need them, we check them. + if(!checkIndex(index, FileStorageNode::TYPE_DIR) || static_cast(mNodes[index])->dir_hash != hash) + { + std::cerr << "(II) removing non existing hash from dir hash list: " << hash << std::endl; + + mDirHashes.erase(it) ; + return false ; + } return true; } -bool InternalFileHierarchyStorage::getIndexFromFileHash(const RsFileHash& hash,DirectoryStorage::EntryIndex& index) const +bool InternalFileHierarchyStorage::getIndexFromFileHash(const RsFileHash& hash,DirectoryStorage::EntryIndex& index) { - std::map::const_iterator it = mFileHashes.find(hash) ; + std::map::iterator it = mFileHashes.find(hash) ; if(it == mFileHashes.end()) return false; index = it->second; + + // make sure the hash actually points to some existing file. If not, remove it. This is a lazy update of file hashes: when we need them, we check them. + if(!checkIndex(it->second, FileStorageNode::TYPE_FILE) || static_cast(mNodes[index])->file_hash != hash) + { + std::cerr << "(II) removing non existing hash from file hash list: " << hash << std::endl; + + mFileHashes.erase(it) ; + return false ; + } + return true; } @@ -226,7 +243,7 @@ bool InternalFileHierarchyStorage::removeDirectory(DirectoryStorage::EntryIndex parent_dir.subdirs[i] = parent_dir.subdirs.back() ; parent_dir.subdirs.pop_back(); - recursRemoveDirectory(indx) ; + recursRemoveDirectory(indx) ; #ifdef DEBUG_DIRECTORY_STORAGE print(); std::string err ; @@ -633,12 +650,12 @@ DirectoryStorage::EntryIndex InternalFileHierarchyStorage::getSubDirIndex(Direct bool InternalFileHierarchyStorage::searchHash(const RsFileHash& hash,std::list& results) { - std::map::const_iterator it = mFileHashes.find(hash); + DirectoryStorage::EntryIndex indx ; - if( it != mFileHashes.end() ) + if(getIndexFromFileHash(hash,indx)) { results.clear(); - results.push_back(it->second) ; + results.push_back(indx) ; return true ; } else @@ -699,55 +716,68 @@ int InternalFileHierarchyStorage::searchTerms(const std::list& term return 0 ; } -bool InternalFileHierarchyStorage::check(std::string& error_string) const // checks consistency of storage. +bool InternalFileHierarchyStorage::check(std::string& error_string) // checks consistency of storage. { // recurs go through all entries, check that all std::vector hits(mNodes.size(),0) ; // count hits of children. Should be 1 for all in the end. Otherwise there's an error. hits[0] = 1 ; // because 0 is never the child of anyone + std::map tmp_hashes ; for(uint32_t i=0;itype() == FileStorageNode::TYPE_DIR) { // stamp the kids - const DirEntry& de = *static_cast(mNodes[i]) ; + DirEntry& de = *static_cast(mNodes[i]) ; - for(uint32_t j=0;j= mNodes.size()) { - error_string = "Node child out of tab!" ; - return false ; + error_string += " - Node child out of tab!" ; + de.subdirs[j] = de.subdirs.back() ; + de.subdirs.pop_back(); } - if(hits[de.subdirs[j]] != 0) + else if(hits[de.subdirs[j]] != 0) { - error_string = "Double hit on a single node" ; - return false; + error_string += " - Double hit on a single node" ; + de.subdirs[j] = de.subdirs.back() ; + de.subdirs.pop_back(); + } + else + { + hits[de.subdirs[j]] = 1; + ++j ; } - hits[de.subdirs[j]] = 1; } - for(uint32_t j=0;j= mNodes.size()) { - error_string = "Node child out of tab!" ; - return false ; + error_string += " - Node child out of tab!" ; + de.subfiles[j] = de.subfiles.back() ; + de.subfiles.pop_back(); } - if(hits[de.subfiles[j]] != 0) + else if(hits[de.subfiles[j]] != 0) { - error_string = "Double hit on a single node" ; - return false; + error_string += " - Double hit on a single node" ; + de.subfiles[j] = de.subfiles.back() ; + de.subfiles.pop_back(); + } + else + { + hits[de.subfiles[j]] = 1; + ++j ; } - hits[de.subfiles[j]] = 1; } - } for(uint32_t i=0;i &results) const ; int searchTerms(const std::list& terms, std::list &results) const ; // does a logical OR between items of the list of terms - bool check(std::string& error_string) const ;// checks consistency of storage. + bool check(std::string& error_string) ;// checks consistency of storage. void print() const;