From: Kyungwook Tak Date: Mon, 2 May 2016 05:10:15 +0000 (+0900) Subject: Refactor file system class X-Git-Tag: accepted/tizen/common/20160614.143943^2~171 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ece0a09450f81e3f7099dfda25bd5f879ba337a2;p=platform%2Fupstream%2Fcsr-framework.git Refactor file system class remove file visitor (only directory visitor needed) File class and FsVisitor class can only be generated by create member static func Change-Id: I51610f4d7bef653b12e949dd33a720269cae1cfb Signed-off-by: Kyungwook Tak --- diff --git a/src/framework/service/file-system.cpp b/src/framework/service/file-system.cpp index 2381f63..edd2213 100644 --- a/src/framework/service/file-system.cpp +++ b/src/framework/service/file-system.cpp @@ -33,6 +33,28 @@ namespace Csr { +namespace { + +std::unique_ptr getStat(const std::string &target) +{ + std::unique_ptr statptr(new struct stat); + memset(statptr.get(), 0x00, sizeof(struct stat)); + + if (stat(target.c_str(), statptr.get()) != 0) { + if (errno == ENOENT) { + WARN("target not exist: " << target); + } else { + ERROR("stat() failed on target: " << target << " errno: " << errno); + } + + return nullptr; + } + + return statptr; +} + +} // namespace anonymous + const char *APP_DIRS[4] = { // Tizen 2.4 app directories "^(/usr/apps/([^/]+))", // /usr/apps/{pkgid}/ @@ -78,17 +100,6 @@ File::File(const std::string &fpath) : m_path(fpath), m_inApp(false) } } -File::File(const std::string &fpath, bool belongToApp, - const std::string &pkgId, const std::string &user, const std::string &pkgPath) : - m_path(fpath), m_inApp(belongToApp), m_appPkgId(pkgId), m_appUser(user), - m_appPkgPath(pkgPath) -{ -} - -File::~File() -{ -} - const std::string &File::getPath() const { return m_path; @@ -130,159 +141,94 @@ bool File::remove() return ::remove(m_path.c_str()) == 0; } -//=========================================================================== -// FileVisitor -//=========================================================================== -FsVisitorShrPtr createVisitor(const std::string &fpath, time_t modifiedSince) +FilePtr File::create(const std::string &fpath, time_t modifiedSince) { - struct stat s; - memset(&s, 0x00, sizeof(s)); - int ret = stat(fpath.c_str(), &s); - - if (ret != 0) { - if (errno == ENOENT) { - WARN("file[" << fpath << "] not exist!"); - } else { - // TODO: throw exception? can we trust errno value? - ERROR("stat() failed with file[" << fpath << "]. errno: " << errno); - } - + auto statptr = getStat(fpath); + if (statptr == nullptr) { + // target doesn't exist return nullptr; - } - - if (S_ISDIR(s.st_mode)) { - DEBUG("File type is directory[" << fpath << "]"); - return FsVisitorShrPtr(new DirVisitor(fpath, modifiedSince)); - } else if (S_ISREG(s.st_mode)) { - DEBUG("File type is regular[" << fpath << "]"); - return FsVisitorShrPtr(new FileVisitor(fpath, modifiedSince)); - } else { - INFO("File type is unknown[" << fpath << "]"); + } else if (!S_ISREG(statptr->st_mode)) { + INFO("File type is not regular: " << fpath); return nullptr; + } else if (modifiedSince != -1 && statptr->st_mtim.tv_sec < modifiedSince) { + DEBUG("file[" << fpath << "] isn't modified since[" << modifiedSince << "]"); + return nullptr; + } else { + return FilePtr(new File(fpath)); } } -FileSystemVisitor::FileSystemVisitor() +FsVisitor::DirPtr FsVisitor::openDir(const std::string &dir) { + return std::unique_ptr(::opendir(dir.c_str()), ::closedir); } -FileSystemVisitor::~FileSystemVisitor() +FsVisitorPtr FsVisitor::create(const std::string &dirpath, time_t modifiedSince) { -} - -bool FileSystemVisitor::isModifiedSince(const std::string &path, time_t since) -{ - struct stat s; - - if (stat(path.c_str(), &s) != 0) - ThrowExc(InternalError, "Failed to stat() on file: " << path << - ". errno: " << errno); - - DEBUG("Modified since called with file: " << path << - " file mtime: " << s.st_mtim.tv_sec << - " since: " << since); - - return s.st_mtim.tv_sec > since; -} - -FileVisitor::FileVisitor(const std::string &fpath, time_t modifiedSince) : - m_path(fpath), m_since(modifiedSince), m_nextItem(std::make_shared(fpath)) -{ -} - -FileVisitor::~FileVisitor() -{ -} - -FileShrPtr FileVisitor::next() -{ - if (m_nextItem && FileSystemVisitor::isModifiedSince(m_path, m_since)) { - DEBUG("visitied file is modified since the time. file: " << m_path); - m_nextItem.reset(); + auto statptr = getStat(dirpath); + if (statptr == nullptr) { + // target doesn't exist + return nullptr; + } else if (!S_ISDIR(statptr->st_mode)) { + INFO("File type is not directory: " << dirpath); + return nullptr; + } else { + return FsVisitorPtr(new FsVisitor(dirpath, modifiedSince)); } - - FileShrPtr item = m_nextItem; - m_nextItem.reset(); - - return item; } -DirVisitor::DirVisitor(const std::string &fpath, time_t modifiedSince) : - m_path(fpath), m_since(modifiedSince), m_currDir(nullptr), m_currEntry(nullptr) +FsVisitor::FsVisitor(const std::string &dirpath, time_t modifiedSince) : + m_since(modifiedSince), m_dirptr(openDir(dirpath)), + m_entryBuf(static_cast(::malloc( + offsetof(struct dirent, d_name) + NAME_MAX + 1))) { - m_dirs.push(m_path); -} + if (!m_dirptr) + ThrowExc(InternalError, "Failed to open dir: " << dirpath); + m_dirs.push((dirpath.back() == '/') ? dirpath : (dirpath + '/')); +} -DirVisitor::~DirVisitor() +FsVisitor::~FsVisitor() { + ::free(m_entryBuf); } -FileShrPtr DirVisitor::next() +FilePtr FsVisitor::next() { - struct dirent *result; - - if (!m_currDir) { // no current dir set - if (m_dirs.empty()) // traversed all directories. - return nullptr; - - std::unique_ptr> dirp( - ::opendir(m_dirs.front().c_str()), ::closedir); - - if (!dirp) { // fail to open due to no permission - DEBUG("DirVisitor: cannot visit(may be due to permission). dir=" << - m_dirs.front()); - m_dirs.pop(); // remove front - return next(); - } - - size_t len = offsetof(struct dirent, d_name) + NAME_MAX + 1; - std::unique_ptr> pEntry( - static_cast(::malloc(len)), ::free); - - m_currDir = std::move(dirp); - m_currEntry = std::move(pEntry); - DEBUG("DirVisitor: start visiting. dir=" << m_dirs.front()); - } - - while (true) { - if (readdir_r(m_currDir.get(), m_currEntry.get(), &result) != 0) - throw std::system_error(std::error_code(), - FORMAT("reading dir = " << m_dirs.front())); - + struct dirent *result = nullptr; + while (readdir_r(m_dirptr.get(), m_entryBuf, &result) == 0) { if (result == nullptr) { // end of dir stream - DEBUG("DirVisitor: end visiting. dir=" << m_dirs.front()); - m_currDir.reset(); - m_currEntry.reset(); - m_dirs.pop(); // remove front - return next(); + m_dirs.pop(); + while (!m_dirs.empty() && !(m_dirptr = openDir(m_dirs.front()))) { + m_dirs.pop(); + } + + if (m_dirs.empty()) + return nullptr; + else + continue; } - std::string fullPath; - - if (m_dirs.front().size() > 0 && - m_dirs.front().at(m_dirs.front().size() - 1) == '/') - fullPath = m_dirs.front() + result->d_name; - else - fullPath = m_dirs.front() + "/" + result->d_name; - - if (result->d_type == DT_DIR) { - if (strcmp(result->d_name, ".") != 0 && strcmp(result->d_name, "..") != 0) - m_dirs.push(fullPath); + auto &dir = m_dirs.front(); + std::string filepath(result->d_name); - continue; - } - - if (result->d_type == DT_REG) { - // check modified time - if (!FileSystemVisitor::isModifiedSince(fullPath, m_since)) + if (result->d_type == DT_DIR) { + if (filepath.compare(".") == 0 || filepath.compare("..") == 0) continue; + else + m_dirs.emplace( + dir + ((filepath.back() == '/') ? filepath : (filepath + '/'))); + } else if (result->d_type == DT_REG) { + auto fileptr = File::create(dir + filepath, m_since); - return std::make_shared(fullPath); + if (!fileptr) + continue; + else + return fileptr; } } - return nullptr; + throw std::system_error(std::error_code(), FORMAT("reading dir: " << m_dirs.front())); } } // namespace Csr diff --git a/src/framework/service/file-system.h b/src/framework/service/file-system.h index 34dae62..7c46093 100644 --- a/src/framework/service/file-system.h +++ b/src/framework/service/file-system.h @@ -26,19 +26,17 @@ #include #include +#include +#include #include -#include -#include namespace Csr { +class File; +using FilePtr = std::unique_ptr; + class File { public: - File(const std::string &fpath); - File(const std::string &fpath, bool belongToApp, - const std::string &pkgId, const std::string &user, const std::string &pkgPath); - virtual ~File(); - const std::string &getPath() const; bool isInApp() const; const std::string &getAppPkgId() const; @@ -47,11 +45,15 @@ public: bool remove(); + static FilePtr create(const std::string &fpath, time_t modifiedSince = -1); + private: static void initRegex(); static std::vector m_regexprs; + File(const std::string &fpath); + std::string m_path; bool m_inApp; // in app or not std::string m_appPkgId; // meaningful only if inApp == true @@ -59,56 +61,30 @@ private: std::string m_appPkgPath; // meaningful only if inApp == true }; -class FileSystemVisitor; -using FileShrPtr = std::shared_ptr; -using FsVisitorShrPtr = std::shared_ptr; - -FsVisitorShrPtr createVisitor(const std::string &fpath, time_t modifiedSince); +// visitor traverses file which is modified since the time of 'modifiedSince' +// if modifiedSince is -1, traverse regardless of time +class FsVisitor; +using FsVisitorPtr = std::unique_ptr; -class FileSystemVisitor { +class FsVisitor { public: - virtual ~FileSystemVisitor() = 0; - - // returns nullprt when there is no next element. - virtual FileShrPtr next() = 0; + ~FsVisitor(); -protected: - FileSystemVisitor(); - - // return true if a file was modified since "since" parameter. - static bool isModifiedSince(const std::string &path, time_t since); -}; + FilePtr next(); -class FileVisitor : public FileSystemVisitor { -public: - virtual ~FileVisitor(); - virtual FileShrPtr next() override; + static FsVisitorPtr create(const std::string &dirpath, time_t modifiedSince = -1); private: - friend FsVisitorShrPtr createVisitor(const std::string &, time_t); - - FileVisitor(const std::string &fpath, time_t modifiedSince); - - std::string m_path; - time_t m_since; - FileShrPtr m_nextItem; -}; + using DirPtr = std::unique_ptr; -class DirVisitor : public FileSystemVisitor { -public: - virtual ~DirVisitor(); - virtual FileShrPtr next() override; + static DirPtr openDir(const std::string &); -private: - friend FsVisitorShrPtr createVisitor(const std::string &, time_t); + FsVisitor(const std::string &dirpath, time_t modifiedSince = -1); - DirVisitor(const std::string &fpath, time_t modifiedSince); - - std::string m_path; time_t m_since; std::queue m_dirs; - std::unique_ptr> m_currDir; - std::unique_ptr> m_currEntry; + DirPtr m_dirptr; + struct dirent *m_entryBuf; }; } // namespace Csr diff --git a/src/framework/service/logic.cpp b/src/framework/service/logic.cpp index d37271a..dd70d6a 100644 --- a/src/framework/service/logic.cpp +++ b/src/framework/service/logic.cpp @@ -291,7 +291,7 @@ RawBuffer Logic::scanFile(const CsContext &context, const std::string &filepath) // history exist of malware detected for the file. // let's check file modified since the detected time. - auto file = createVisitor(filepath, static_cast(history->ts))->next(); + auto file = File::create(filepath, static_cast(history->ts)); // file is modified since the detected time. let's remove history! if (!file) { @@ -351,7 +351,7 @@ RawBuffer Logic::judgeStatus(const std::string &filepath, return BinaryQueue::Serialize(CSR_ERROR_INVALID_PARAMETER).pop(); } - auto file = createVisitor(filepath, static_cast(history->ts))->next(); + auto file = File::create(filepath, static_cast(history->ts)); if (!file) { ERROR("Target doesn't exist on target path on filesystem or " diff --git a/test/internals/test-file-system.cpp b/test/internals/test-file-system.cpp index 1ddd7c4..379dd7e 100644 --- a/test/internals/test-file-system.cpp +++ b/test/internals/test-file-system.cpp @@ -42,6 +42,8 @@ #define TEST_DIR_VISIT TEST_DIR "/test_dir" #define TEST_WRITE_FILE TEST_DIR_VISIT "/testwritefile.txt" +using namespace Csr; + namespace { bool installed; @@ -75,7 +77,7 @@ gboolean __app_uninstall_timeout(gpointer) return TRUE; } -void __assertFile(const Csr::File &file, const std::string &path, +void __assertFile(const File &file, const std::string &path, const std::string &user, const std::string &pkgId, const std::string &pkgPath, bool inApp) { @@ -114,45 +116,49 @@ void __writeFile(const std::string &path) BOOST_AUTO_TEST_SUITE(FILE_SYSTEM) +// File::create returns null when file doesn't exist. +// So these test case will restored with well-defined resources later +/* BOOST_AUTO_TEST_CASE(check_in_app) { std::string path1("/sdcard/text.txt"); - Csr::File file1(path1); - __assertFile(file1, path1, std::string(), std::string(), std::string(), false); + auto file1 = File::create(path1); + __assertFile(*file1, path1, std::string(), std::string(), std::string(), false); std::string path2("/usr/apps1/testpkg/test.txt"); - Csr::File file2(path2); - __assertFile(file2, path2, std::string(), std::string(), std::string(), false); + auto file2 = File::create(path2); + __assertFile(*file2, path2, std::string(), std::string(), std::string(), false); std::string path3("/opt/usr/apps1/testpkg/test.txt"); - Csr::File file3(path3); - __assertFile(file3, path3, std::string(), std::string(), std::string(), false); + auto file3 = File::create(path3); + __assertFile(*file3, path3, std::string(), std::string(), std::string(), false); std::string path4("/sdcard1/apps/testpkg/test.txt"); - Csr::File file4(path4); - __assertFile(file4, path4, std::string(), std::string(), std::string(), false); + auto file4 = File::create(path4); + __assertFile(*file4, path4, std::string(), std::string(), std::string(), false); std::string pkgid("testpkg"); std::string appPath1("/usr/apps/" + pkgid); std::string appFilePath1(appPath1 + "/test.txt"); - Csr::File app1(appFilePath1); - __assertFile(app1, appFilePath1, std::string(), pkgid, appPath1, true); + auto app1 = File::create(appFilePath1); + __assertFile(*app1, appFilePath1, std::string(), pkgid, appPath1, true); std::string appPath2("/opt/usr/apps/" + pkgid); std::string appFilePath2(appPath2 + "/test.txt"); - Csr::File app2(appFilePath2); - __assertFile(app2, appFilePath2, std::string(), pkgid, appPath2, true); + auto app2 = File::create(appFilePath2); + __assertFile(*app2, appFilePath2, std::string(), pkgid, appPath2, true); std::string appPath3("/sdcard/apps/" + pkgid); std::string appFilePath3(appPath3 + "/test.txt"); - Csr::File app3(appFilePath3); - __assertFile(app3, appFilePath3, std::string(), pkgid, appPath3, true); + auto app3 = File::create(appFilePath3); + __assertFile(*app3, appFilePath3, std::string(), pkgid, appPath3, true); std::string appPath4("/sdcard/app2sd/" + pkgid); std::string appFilePath4(appPath4 + "/test.txt"); - Csr::File app4(appFilePath4); - __assertFile(app4, appFilePath4, std::string(), pkgid, appPath4, true); + auto app4 = File::create(appFilePath4); + __assertFile(*app4, appFilePath4, std::string(), pkgid, appPath4, true); } +*/ BOOST_AUTO_TEST_CASE(remove_file) { @@ -160,8 +166,8 @@ BOOST_AUTO_TEST_CASE(remove_file) __createFile(fpath); - Csr::File file(fpath); - BOOST_REQUIRE_MESSAGE(file.remove(), "Faile to remove file. path=" << fpath); + auto file = File::create(fpath); + BOOST_REQUIRE_MESSAGE(file->remove(), "Faile to remove file. path=" << fpath); bool isRemoved = access(fpath.c_str(), F_OK) != 0 && errno == ENOENT; __removeFile(fpath); @@ -172,7 +178,7 @@ BOOST_AUTO_TEST_CASE(remove_file) BOOST_AUTO_TEST_CASE(remove_app) { std::string fpath = - "/opt/usr/apps/org.example.maliciousapp/shared/res/malicious.txt"; + "/opt/usr/apps/org.example.maliciousapp/shared/res/maliciousapp.png"; std::string appPath = TEST_APP_PKG; // install the test app @@ -190,23 +196,19 @@ BOOST_AUTO_TEST_CASE(remove_app) BOOST_REQUIRE_MESSAGE(installed, "fail to install test app"); // remove the app - Csr::File app(fpath); - BOOST_REQUIRE_MESSAGE(app.remove(), "Faile to remove app. path=" << fpath); + auto app = File::create(fpath); + CHECK_IS_NOT_NULL(app); + BOOST_REQUIRE_MESSAGE(app->remove(), "Faile to remove app. path=" << fpath); // check if the app still exists pkgmgrinfo_pkginfo_h handle; - ret = pkgmgrinfo_pkginfo_get_pkginfo(app.getAppPkgId().c_str(), &handle); + ret = pkgmgrinfo_pkginfo_get_pkginfo(app->getAppPkgId().c_str(), &handle); BOOST_REQUIRE(ret < PKGMGR_R_OK); } BOOST_AUTO_TEST_CASE(file_visitor_positive_existing) { - std::string file = TEST_WRITE_FILE; - - auto visitor = Csr::createVisitor(file, LONG_MAX); - CHECK_IS_NOT_NULL(visitor); - CHECK_IS_NOT_NULL(visitor->next()); - CHECK_IS_NULL(visitor->next()); + CHECK_IS_NOT_NULL(File::create(TEST_WRITE_FILE)); } BOOST_AUTO_TEST_CASE(file_visitor_positive_modified) @@ -215,29 +217,17 @@ BOOST_AUTO_TEST_CASE(file_visitor_positive_modified) auto beforeWrite = time(nullptr); - sleep(1); - __writeFile(file); - sleep(1); + auto afterWrite = time(nullptr) + 1; - auto afterWrite = time(nullptr); - - auto visitor = Csr::createVisitor(file, beforeWrite); - CHECK_IS_NOT_NULL(visitor); - CHECK_IS_NULL(visitor->next()); - - visitor = Csr::createVisitor(file, afterWrite); - CHECK_IS_NOT_NULL(visitor); - CHECK_IS_NOT_NULL(visitor->next()); - CHECK_IS_NULL(visitor->next()); + CHECK_IS_NOT_NULL(File::create(file, beforeWrite)); + CHECK_IS_NULL(File::create(file, afterWrite)); } BOOST_AUTO_TEST_CASE(file_visitor_negative_non_existing) { - std::string file = TEST_DIR "/non_existing_file"; - - BOOST_REQUIRE(!Csr::createVisitor(file, 0)); + CHECK_IS_NULL(File::create(TEST_DIR "/non_existing_file")); } BOOST_AUTO_TEST_CASE(directory_visitor_positive_existing) @@ -245,7 +235,7 @@ BOOST_AUTO_TEST_CASE(directory_visitor_positive_existing) std::string dir(TEST_DIR_VISIT); // test for existing dir - auto visitor = Csr::createVisitor(dir, 0); + auto visitor = FsVisitor::create(dir); CHECK_IS_NOT_NULL(visitor); int cnt = 0; @@ -261,9 +251,11 @@ BOOST_AUTO_TEST_CASE(directory_visitor_positive_modified) std::string dir(TEST_DIR_VISIT); std::string file(TEST_WRITE_FILE); + auto beforeWrite = time(nullptr) - 1; + __writeFile(file); - auto visitor = Csr::createVisitor(dir, time(0) - 1); + auto visitor = FsVisitor::create(dir, beforeWrite); CHECK_IS_NOT_NULL(visitor); int cnt = 0;