From 27fca410346f874bf5060835df072be54e0948fa Mon Sep 17 00:00:00 2001 From: Kyungwook Tak Date: Wed, 29 Jun 2016 17:31:59 +0900 Subject: [PATCH] Change file visitor to dfs style not to have queue Change-Id: I2bb154ce573468cbaab8fb0d32aae8f0e746ef20 Signed-off-by: Kyungwook Tak --- src/framework/service/cs-logic.cpp | 26 +++++----- src/framework/service/file-system.cpp | 95 +++++++++++++++++------------------ src/framework/service/file-system.h | 20 +++++--- test/internals/test-file-system.cpp | 35 +++++++------ 4 files changed, 90 insertions(+), 86 deletions(-) diff --git a/src/framework/service/cs-logic.cpp b/src/framework/service/cs-logic.cpp index bcfa7b2..b02f263 100644 --- a/src/framework/service/cs-logic.cpp +++ b/src/framework/service/cs-logic.cpp @@ -216,12 +216,9 @@ CsDetectedPtr CsLogic::scanAppDelta(const std::string &pkgPath, const std::strin auto t = this->m_loader->getEngineLatestUpdateTime(c); auto lastScanTime = this->m_db->getLastScanTime(pkgPath, t); - // traverse files in app and take which is more danger than riskiest - auto visitor = FsVisitor::create(pkgPath, false, lastScanTime); - CsDetectedPtr riskiest; - - while (auto file = visitor->next()) { + // traverse files in app and take which is more danger than riskiest + auto visitor = FsVisitor::create([&](const FilePtr &file) { DEBUG("Scan file by engine: " << file->getPath()); auto timestamp = ::time(nullptr); @@ -233,7 +230,7 @@ CsDetectedPtr CsLogic::scanAppDelta(const std::string &pkgPath, const std::strin if (lastScanTime != -1) this->m_db->deleteDetectedByFilepathOnPath(file->getPath()); - continue; + return; } INFO("New malware detected on file: " << file->getPath()); @@ -252,7 +249,9 @@ CsDetectedPtr CsLogic::scanAppDelta(const std::string &pkgPath, const std::strin *riskiest = std::move(candidate); riskiestPath = file->getPath(); } - } + }, pkgPath, false, lastScanTime); + + visitor->run(); this->m_db->insertLastScanTime(pkgPath, this->m_dataVersion, starttime); @@ -488,21 +487,20 @@ RawBuffer CsLogic::getScannableFiles(const std::string &dir, const std::function auto lastScanTime = this->m_db->getLastScanTime(targetdir, since); - auto visitor = FsVisitor::create(targetdir, true, lastScanTime); - - isCancelled(); - StrSet fileset; - while (auto file = visitor->next()) { + auto visitor = FsVisitor::create([&](const FilePtr &file) { isCancelled(); DEBUG("Scannable item: " << file->getName()); fileset.insert(file->getName()); - } + }, targetdir, true, lastScanTime); - if (lastScanTime != -1) { + visitor->run(); + isCancelled(); + + if (lastScanTime != -1) { // for case: scan history exist and not modified. for (auto &row : this->m_db->getDetectedAllByNameOnDir(targetdir, since)) { isCancelled(); diff --git a/src/framework/service/file-system.cpp b/src/framework/service/file-system.cpp index c08f345..934654a 100644 --- a/src/framework/service/file-system.cpp +++ b/src/framework/service/file-system.cpp @@ -227,10 +227,13 @@ FilePtr File::createInternal(const std::string &fpath, const FilePtr &parentdir, FsVisitor::DirPtr FsVisitor::openDir(const std::string &dir) { - return std::unique_ptr(::opendir(dir.c_str()), ::closedir); + return std::unique_ptr( + (isInBlackList(dir) ? nullptr : ::opendir(dir.c_str())), ::closedir); } -FsVisitorPtr FsVisitor::create(const std::string &dirpath, bool isBasedOnName, time_t modifiedSince) +FsVisitorPtr FsVisitor::create(TargetHandler &&targetHandler, + const std::string &dirpath, bool isBasedOnName, + time_t modifiedSince) { auto statptr = getStat(dirpath); if (statptr == nullptr) @@ -239,18 +242,18 @@ FsVisitorPtr FsVisitor::create(const std::string &dirpath, bool isBasedOnName, t else if (!S_ISDIR(statptr->st_mode)) ThrowExc(CSR_ERROR_FILE_SYSTEM, "file type is not directory: " << dirpath); else - return FsVisitorPtr(new FsVisitor(dirpath, isBasedOnName, modifiedSince)); + return FsVisitorPtr(new FsVisitor(std::move(targetHandler), dirpath, + isBasedOnName, modifiedSince)); } -FsVisitor::FsVisitor(const std::string &dirpath, bool isBasedOnName, time_t modifiedSince) : - m_since(modifiedSince), m_dirptr(nullptr, ::closedir), - m_entryBuf(static_cast(::malloc(offsetof(struct dirent, d_name) + NAME_MAX + 1))), - m_isDone(true), m_isBasedOnName(isBasedOnName) +FsVisitor::FsVisitor(TargetHandler &&targetHandler, + const std::string &dirpath, bool isBasedOnName, time_t modifiedSince) : + m_targetHandler(std::move(targetHandler)), m_path(dirpath), + m_since(modifiedSince), m_isDone(true), m_isBasedOnName(isBasedOnName), + m_entryBuf(static_cast(::malloc(offsetof(struct dirent, d_name) + NAME_MAX + 1))) { if (this->m_entryBuf == nullptr) throw std::bad_alloc(); - - this->m_dirs.emplace(File::create(dirpath, nullptr)); } FsVisitor::~FsVisitor() @@ -258,39 +261,19 @@ FsVisitor::~FsVisitor() ::free(this->m_entryBuf); } -FilePtr FsVisitor::next() +void FsVisitor::run(const DirPtr &dirptr, const FilePtr ¤tdir) { + struct dirent *result = nullptr; while (true) { - if (this->m_isDone) { - while (!this->m_dirs.empty() && - !(this->m_dirptr = openDir(this->m_dirs.front()->getPath())) && - isInBlackList(this->m_dirs.front()->getPath())) - this->m_dirs.pop(); - - if (this->m_dirs.empty()) { - this->m_currentdir.reset(); - this->m_isDone = true; - return nullptr; - } else { - this->m_currentdir = std::move(this->m_dirs.front()); - this->m_dirs.pop(); - this->m_isDone = false; - DEBUG("dir opened: " << this->m_currentdir->getPath()); - } - } - - struct dirent *result = nullptr; - const auto &parent_dirpath = this->m_currentdir->getPath(); - if (::readdir_r(this->m_dirptr.get(), this->m_entryBuf, &result) != 0) { - ERROR("readdir_r error on dir: " << parent_dirpath << - " with errno: " << errno << ". Silently ignore this error & dir stream" - " to reduce side-effect of traversing all the other file systems."); - this->m_isDone = true; - continue; + auto ret = ::readdir_r(dirptr.get(), this->m_entryBuf, &result); + if (ret != 0) { + WARN("readdir_r error on dir: " << currentdir->getPath() << + " with errno: " << errno << ". Silently ignore this error & dir stream" + " to reduce side-effect of traversing all the other file systems."); + break; } else if (result == nullptr) { - DEBUG("End of stream of dir: " << parent_dirpath); - this->m_isDone = true; - continue; + DEBUG("End of stream of dir: " << currentdir->getPath()); + break; } const auto &name = result->d_name; @@ -299,6 +282,7 @@ FilePtr FsVisitor::next() if (name_size == 0) continue; + const auto &parent_dirpath = currentdir->getPath(); auto fullpath = (parent_dirpath.back() == '/') ? (parent_dirpath + name) : (parent_dirpath + "/" + name); @@ -307,9 +291,15 @@ FilePtr FsVisitor::next() (name_size == 2 && name[0] == '.' && name[1] == '.')) continue; - FilePtr dirptr; + auto ndirptr = openDir(fullpath); + if (ndirptr == nullptr) { + WARN("Failed to open dir: " << fullpath); + continue; + } + + FilePtr ncurrentdir; try { - dirptr = File::create(fullpath, this->m_currentdir); + ncurrentdir = File::create(fullpath, currentdir); } catch (const Exception &e) { if (e.error() == CSR_ERROR_FILE_DO_NOT_EXIST) { WARN("Perm denied to create file on pkg path: " << fullpath); @@ -319,18 +309,17 @@ FilePtr FsVisitor::next() } } - if (this->m_isBasedOnName && dirptr->isInApp()) - return dirptr; + if (this->m_isBasedOnName && ncurrentdir->isInApp()) + this->m_targetHandler(ncurrentdir); - DEBUG("push dir to dirs queue: " << fullpath); - this->m_dirs.emplace(std::move(dirptr)); + DEBUG("recurse dir : " << fullpath); + this->run(ndirptr, ncurrentdir); } else if (result->d_type == DT_REG) { try { - auto fileptr = File::createIfModified( - fullpath, this->m_currentdir, this->m_since); + auto fileptr = File::createIfModified(fullpath, currentdir, this->m_since); if (fileptr) - return fileptr; + this->m_targetHandler(fileptr); } catch (const Exception &e) { if (e.error() == CSR_ERROR_FILE_DO_NOT_EXIST) WARN("file not exist: " << fullpath << " msg: " << e.what()); @@ -342,8 +331,16 @@ FilePtr FsVisitor::next() } } } +} + +void FsVisitor::run() +{ + auto dirptr = openDir(this->m_path); + auto currentdir = File::create(this->m_path, nullptr); + + INFO("Visiting files start from dir: " << this->m_path); - return nullptr; + this->run(dirptr, currentdir); } } // namespace Csr diff --git a/src/framework/service/file-system.h b/src/framework/service/file-system.h index add4ad8..70f6ca8 100644 --- a/src/framework/service/file-system.h +++ b/src/framework/service/file-system.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -135,28 +136,33 @@ using FsVisitorPtr = std::unique_ptr; class FsVisitor { public: + using TargetHandler = std::function; + ~FsVisitor(); - FilePtr next(); + void run(); // throws FileNotExist and FileSystemError - static FsVisitorPtr create(const std::string &dirpath, bool isBasedOnName, + static FsVisitorPtr create(TargetHandler &&targetHandler, + const std::string &dirpath, bool isBasedOnName, time_t modifiedSince = -1); private: using DirPtr = std::unique_ptr; + void run(const DirPtr &dirptr, const FilePtr ¤tdir); + static DirPtr openDir(const std::string &); - FsVisitor(const std::string &dirpath, bool isBasedOnName, time_t modifiedSince = -1); + FsVisitor(TargetHandler &&targetHandler, const std::string &dirpath, + bool isBasedOnName, time_t modifiedSince = -1); + TargetHandler m_targetHandler; + std::string m_path; time_t m_since; - std::queue m_dirs; - DirPtr m_dirptr; - struct dirent *m_entryBuf; - FilePtr m_currentdir; bool m_isDone; bool m_isBasedOnName; + struct dirent *m_entryBuf; }; } // namespace Csr diff --git a/test/internals/test-file-system.cpp b/test/internals/test-file-system.cpp index b0aba7e..b848473 100644 --- a/test/internals/test-file-system.cpp +++ b/test/internals/test-file-system.cpp @@ -181,8 +181,8 @@ BOOST_AUTO_TEST_CASE(file_visitor_positive_modified) // if modifiedSince is same to modified time in file stat, // the file will be regarded as ''not modified'' no File::create returns null. - CHECK_IS_NOT_NULL(File::create(file, nullptr, beforeWrite)); - CHECK_IS_NULL(File::create(file, nullptr, afterWrite)); + CHECK_IS_NOT_NULL(File::createIfModified(file, nullptr, beforeWrite)); + CHECK_IS_NULL(File::createIfModified(file, nullptr, afterWrite)); } BOOST_AUTO_TEST_CASE(file_visitor_negative_non_existing) @@ -194,16 +194,17 @@ BOOST_AUTO_TEST_CASE(directory_visitor_positive_existing) { std::string dir(TEST_DIR_VISIT); + int cnt = 0; + // test for existing dir - auto visitor = FsVisitor::create(dir, true); + auto visitor = FsVisitor::create([&](const FilePtr &) { + ++cnt; + }, dir, true); CHECK_IS_NOT_NULL(visitor); - int cnt = 0; + visitor->run(); - while (visitor->next()) - cnt++; - - ASSERT_IF(cnt, 8); + ASSERT_IF(cnt, 9); } BOOST_AUTO_TEST_CASE(directory_visitor_positive_modified) @@ -215,24 +216,26 @@ BOOST_AUTO_TEST_CASE(directory_visitor_positive_modified) __writeFile(file); - auto visitor = FsVisitor::create(dir, true, beforeWrite); - CHECK_IS_NOT_NULL(visitor); - int cnt = 0; - while (visitor->next()) - cnt++; + auto visitor = FsVisitor::create([&](const FilePtr &) { + ++cnt; + }, dir, true, beforeWrite); + CHECK_IS_NOT_NULL(visitor); + + visitor->run(); ASSERT_IF(cnt, 1); } BOOST_AUTO_TEST_CASE(app_directory_visitor_positive) { - auto visitor = FsVisitor::create(TEST_DIR_APPS(), true); + auto visitor = FsVisitor::create([&](const FilePtr &file) { + BOOST_MESSAGE("visit target name: " << file->getPath()); + }, TEST_DIR_APPS(), true); CHECK_IS_NOT_NULL(visitor); - while (auto file = visitor->next()) - BOOST_MESSAGE("visit target name: " << file->getPath()); + visitor->run(); } BOOST_AUTO_TEST_SUITE_END() -- 2.7.4