Change file visitor to dfs style not to have queue 74/77274/1
authorKyungwook Tak <k.tak@samsung.com>
Wed, 29 Jun 2016 08:31:59 +0000 (17:31 +0900)
committerKyungwook Tak <k.tak@samsung.com>
Wed, 29 Jun 2016 08:32:20 +0000 (17:32 +0900)
Change-Id: I2bb154ce573468cbaab8fb0d32aae8f0e746ef20
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>
src/framework/service/cs-logic.cpp
src/framework/service/file-system.cpp
src/framework/service/file-system.h
test/internals/test-file-system.cpp

index bcfa7b2..b02f263 100644 (file)
@@ -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();
index c08f345..934654a 100644 (file)
@@ -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<DIR, int(*)(DIR *)>(::opendir(dir.c_str()), ::closedir);
+       return std::unique_ptr<DIR, int(*)(DIR *)>(
+               (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<struct dirent *>(::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<struct dirent *>(::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 &currentdir)
 {
+       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
index add4ad8..70f6ca8 100644 (file)
@@ -24,6 +24,7 @@
 #include <string>
 #include <memory>
 #include <queue>
+#include <functional>
 
 #include <cstddef>
 #include <ctime>
@@ -135,28 +136,33 @@ using FsVisitorPtr = std::unique_ptr<FsVisitor>;
 
 class FsVisitor {
 public:
+       using TargetHandler = std::function<void(FilePtr &)>;
+
        ~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<DIR, int(*)(DIR *)>;
 
+       void run(const DirPtr &dirptr, const FilePtr &currentdir);
+
        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<FilePtr> m_dirs;
-       DirPtr m_dirptr;
-       struct dirent *m_entryBuf;
-       FilePtr m_currentdir;
        bool m_isDone;
        bool m_isBasedOnName;
+       struct dirent *m_entryBuf;
 };
 
 } // namespace Csr
index b0aba7e..b848473 100644 (file)
@@ -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()