From 9c998cf4fa60c7c32a57fd8ef8f3271712b95b39 Mon Sep 17 00:00:00 2001 From: Cao Chuan Date: Mon, 12 Jun 2023 17:31:01 +0800 Subject: [PATCH 01/12] Add new watcher style Signed-off-by: Cao Chuan --- .../mod_fs/class_watcher/watcher_entity.cpp | 429 ++++++++++++++++-- .../src/mod_fs/class_watcher/watcher_entity.h | 56 ++- .../class_watcher/watcher_n_exporter.cpp | 7 +- .../kits/js/src/mod_fs/properties/watcher.cpp | 42 +- 4 files changed, 466 insertions(+), 68 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index 1500dfcfd..d629465ea 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -14,85 +14,454 @@ */ #include "watcher_entity.h" +#include #include +#include +#include #include +#include #include "filemgmt_libhilog.h" #include "uv.h" + namespace OHOS::FileManagement::ModuleFileIO { using namespace OHOS::FileManagement::LibN; -FileWatcher::FileWatcher() {} +using namespace std; + +namespace { + constexpr int64_t SELECT_TIME_OUT_US = 100000; + constexpr int64_t STOP_WATCHER_TIME_OUT_US = 120000; + const vector EVENT_LIST = { IN_ACCESS, IN_MODIFY, IN_ATTRIB, IN_CLOSE_WRITE, IN_CLOSE_NOWRITE, + IN_OPEN, IN_MOVED_FROM, IN_MOVED_TO, IN_CREATE, IN_DELETE, + IN_DELETE_SELF, IN_MOVE_SELF }; +} -FileWatcher::~FileWatcher() {} +shared_ptr FileWatcher::instance_ = nullptr; +mutex FileWatcher::mutex_; +mutex FileWatcher::watchMutex_; + +shared_ptr FileWatcher::GetInstance() +{ + if (instance_ == nullptr) { + lock_guard lock(mutex_); + if (instance_ == nullptr) { + instance_ = shared_ptr(new FileWatcher()); + } + } + return instance_; +} -bool FileWatcher::InitNotify(int &fd) +int32_t FileWatcher::GetNotifyId() { - fd = inotify_init(); - if (fd < 0) { + return notifyFd_; +} + +bool FileWatcher::InitNotify() +{ + notifyFd_ = inotify_init(); + if (notifyFd_ < 0) { HILOGE("Failed to init notify fail errCode:%{public}d", errno); return false; } return true; } -bool FileWatcher::StartNotify(WatcherInfoArg &arg) +vector GetEvents(const uint32_t &watchEvent) { - int wd = inotify_add_watch(arg.fd, arg.filename.c_str(), arg.events); + vector eventList; + for (auto event : EVENT_LIST) { + if (watchEvent & event) { + eventList.push_back(event); + } + } + return eventList; +} + +vector FindUnWatchedEvent(const vector &allEvents, const uint32_t &event) +{ + vector unWatchedEvents; + vector watchEvents = GetEvents(event); + for (auto event: watchEvents) { + auto iter = find(allEvents.begin(), allEvents.end(), event); + if (iter == allEvents.end()) { + unWatchedEvents.push_back(event); + } + } + return unWatchedEvents; +} + +bool FileWatcher::CheckEventWatched(const string &fileName, const uint32_t &event) +{ + auto iter = fileNameEventMap_.find(fileName); + if (iter == fileNameEventMap_.end()) { + return false; + } + vector allEvents = iter->second; + vector unWatchedEvents = FindUnWatchedEvent(allEvents, event); + if (unWatchedEvents.size() == 0) { + return true; + } + + return false; +} + +bool FileWatcher::CheckEventValid(const uint32_t &event) +{ + vector events = GetEvents(event); + return events.size() > 0 ? true : false; +} + +bool FileWatcher::StartNotify(shared_ptr &arg) +{ + lock_guard lock(watchMutex_); + if (notifyFd_ < 0) { + HILOGE("Failed to start notify fail notifyFd_:%{public}d", notifyFd_); + return false; + } + + if (!CheckEventValid(arg->events)) { + HILOGE("Failed to start notify, the watch event:%{public}x is wrong", arg->events); + errno = E_INVAL; + return false; + } + + if (CheckEventWatched(arg->fileName, arg->events)) { + for (auto iter = wdFileNameMap_.begin(); iter != wdFileNameMap_.end(); iter++) { + if (iter->second == arg->fileName) { + arg->wd = iter->first; + return true; + } + } + return false; + } + vector watchEvents = ParseEvent(arg->fileName.c_str(), arg->events); + uint32_t watchEvent = 0; + for (auto event: watchEvents) { + watchEvent |= event; + } + int wd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); if (wd < 0) { HILOGE("Failed to start notify fail errCode:%{public}d", errno); return false; } - arg.wd = wd; - run_ = true; + arg->wd = wd; + AddWatcherWdFileNameMap(wd, arg->fileName); return true; } -bool FileWatcher::StopNotify(const WatcherInfoArg &arg) +bool FileWatcher::FindWatcherInfo(const std::string &fileName, const uint32_t &event) +{ + auto iter = watcherInfoMap_.find(fileName); + if (iter == watcherInfoMap_.end()) { + return false; + } + vector> args = iter->second; + for (auto arg : args) { + if (arg->events == event) { + return true; + } + } + return false; +} + +void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) +{ + auto watcherInfosIter = watcherInfoMap_.find(arg->fileName); + if (watcherInfosIter == watcherInfoMap_.end()) { + return; + } + auto args = watcherInfosIter->second; + vector watchedEvent; + for (auto arg : args) { + uint32_t event = arg->events; + vector events = GetEvents(event); + watchedEvent.insert(watchedEvent.end(), events.begin(), events.end()); + } + sort(watchedEvent.begin(), watchedEvent.end()); + auto iter = unique(watchedEvent.begin(), watchedEvent.end()); + watchedEvent.erase(iter, watchedEvent.end()); + map>::iterator fileIter; + uint32_t removeEvent = arg->events; + vector removeEvents = GetEvents(removeEvent); + for (auto toRemoveEvent : removeEvents) { + auto watchedIter = find(watchedEvent.begin(), watchedEvent.end(), toRemoveEvent); + if (watchedIter != watchedEvent.end()) { + continue; + } + fileIter = fileNameEventMap_.find(arg->fileName); + if (fileIter == fileNameEventMap_.end()) { + continue; + } + auto eventIter = find(fileIter->second.begin(), fileIter->second.end(), toRemoveEvent); + if (eventIter == fileIter->second.end()) { + continue; + } + fileIter->second.erase(eventIter); + } + + if (fileIter->second.size() == 0) { + fileNameEventMap_.erase(fileIter); + } +} + +bool FileWatcher::NotifyToWatchNewEvents(const shared_ptr &arg) { - run_ = false; - if (arg.events == IN_DELETE_SELF) { - close(arg.fd); + if (FindWatcherInfo(arg->fileName, arg->events)) { + return true; + } + RemoveEventFromMap(arg); + auto eventIter = fileNameEventMap_.find(arg->fileName); + if (eventIter == fileNameEventMap_.end()) { return true; } - if (inotify_rm_watch(arg.fd, arg.wd) == -1) { - HILOGE("Failed to stop notify fail errCode:%{public}d", errno); + vector watchEvents = eventIter->second; + uint32_t watchEvent = 0; + for (auto event : watchEvents) { + watchEvent |= event; + } + int newWd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); + + if (newWd < 0) { + HILOGE("Failed to start new notify fail errCode:%{public}d", errno); + return false; + } else { + if (newWd != arg->wd) { + HILOGE("New notify wd is error"); + return false; + } + AddWatcherWdFileNameMap(newWd, arg->fileName); + return true; + } +} + +bool FileWatcher::FindWatcherByFileName(const shared_ptr &arg) +{ + auto iter = watcherInfoMap_.find(arg->fileName); + if (iter == watcherInfoMap_.end()) { return false; } - close(arg.fd); return true; } -void FileWatcher::HandleEvent(WatcherInfoArg &arg, const struct inotify_event *event, WatcherCallback callback) +bool FileWatcher::StopNotify(const shared_ptr &arg) { - if (event->wd != arg.wd) { - return; + unique_lock lock(watchMutex_); + if (notifyFd_ < 0) { + HILOGE("Failed to stop notify fail notifyFd_:%{public}d", notifyFd_); + return false; + } + RemoveWatcherInfo(arg); + if (FindWatcherByFileName(arg)) { + bool ret = NotifyToWatchNewEvents(arg); + if (!ret) { + return false; + } + } else { + auto iter = fileNameEventMap_.find(arg->fileName); + if (iter != fileNameEventMap_.end()) { + fileNameEventMap_.erase(iter); + } } - std::string fileName = arg.filename; - if (event->len > 0) { - fileName.append(std::string("/")); - fileName.append(std::string(event->name)); + if (GetWatcherInfoSize(arg->wd) == 0) { + if (inotify_rm_watch(notifyFd_, arg->wd) == -1) { + auto iter = wdFileNameMap_.find(arg->wd); + if (iter == wdFileNameMap_.end()) { + return false; + } + string filename = iter->second; + if (access(filename.c_str(), F_OK) == 0) { + HILOGE("Failed to stop notify fail errCode:%{public}d", errno); + RemoveWatcherWdFileNameMap(arg->wd); + return false; + } + } + RemoveWatcherWdFileNameMap(arg->wd); + if (watcherInfoMap_.size() == 0) { + close(notifyFd_); + notifyFd_ = -1; + run_ = false; + stopWatcherCv_.wait_for(lock, chrono::milliseconds(STOP_WATCHER_TIME_OUT_US)); + } + return true; } - callback(arg.env, arg.nRef, fileName, event->mask, event->cookie); + return true; } -void FileWatcher::GetNotifyEvent(WatcherInfoArg &arg, WatcherCallback callback) +void FileWatcher::GetNotifyEvent(shared_ptr &arg, WatcherCallback callback) { + if (run_) { + return; + } + + run_ =true; char buf[BUF_SIZE] = {0}; struct inotify_event *event = nullptr; while (run_) { - int fd = arg.fd; + if (notifyFd_ < 0) { + HILOGE("Failed to run Listener Thread because notifyFd_:%{public}d", notifyFd_); + break; + } fd_set fds; FD_ZERO(&fds); - FD_SET(fd, &fds); - if (select(fd + 1, &fds, nullptr, nullptr, nullptr) > 0) { + FD_SET(notifyFd_, &fds); + timeval timeout{.tv_sec = 0, .tv_usec = SELECT_TIME_OUT_US}; + if (select(notifyFd_ + 1, &fds, nullptr, nullptr, &timeout) > 0) { int len, index = 0; - while (((len = read(fd, &buf, sizeof(buf))) < 0) && (errno == EINTR)) {}; + while (((len = read(notifyFd_, &buf, sizeof(buf))) < 0) && (errno == EINTR)) {}; while (index < len) { event = reinterpret_cast(buf + index); - HandleEvent(arg, event, callback); + NotifyEvent(event, callback); index += sizeof(struct inotify_event) + event->len; } } } + + if (!run_) { + stopWatcherCv_.notify_one(); + } +} + +void FileWatcher::AddWatcherInfo(const string &fileName, const shared_ptr &arg) +{ + WatcherInfoMap::iterator it = watcherInfoMap_.find(fileName); + if (it == watcherInfoMap_.end()) { + vector> watcherInfoVec = { arg }; + watcherInfoMap_[fileName] = watcherInfoVec; + return; + } + + auto watcherInfos = it->second; + for (auto &info : watcherInfos) { + bool isSame = false; + napi_strict_equals(info->env, info->nRef.Deref(info->env).val_, arg->nRef.Deref(arg->env).val_, &isSame); + if (isSame && info->events == arg->events) { + return; + } + } + it->second.push_back(arg); +} + +vector FileWatcher::ParseEvent(const string &fileName, const uint32_t &watcherEvent) +{ + auto iter = fileNameEventMap_.find(fileName); + if (iter == fileNameEventMap_.end()) { + vector events = GetEvents(watcherEvent); + fileNameEventMap_[fileName] = events; + return events; + } else { + vector notWatchedEvents = FindUnWatchedEvent(iter->second, watcherEvent); + if (notWatchedEvents.size() > 0) { + vector newEvents; + newEvents.insert(newEvents.end(), iter->second.begin(), iter->second.end()); + newEvents.insert(newEvents.end(), notWatchedEvents.begin(), notWatchedEvents.end()); + sort(newEvents.begin(), newEvents.end()); + auto newEventIter = unique(newEvents.begin(), newEvents.end()); + newEvents.erase(newEventIter, newEvents.end()); + fileNameEventMap_[fileName] = newEvents; + return newEvents; + } else { + return iter->second; + } + } +} + +void FileWatcher::RemoveWatcherInfo(const shared_ptr &arg) +{ + auto fileNameIter = wdFileNameMap_.find(arg->wd); + if (fileNameIter == wdFileNameMap_.end()) { + return; + } + string fileName = fileNameIter->second; + auto iter = watcherInfoMap_.find(fileName); + if (iter == watcherInfoMap_.end()) { + return; + } + auto infoArgs = iter->second; + for (size_t i = 0; i < infoArgs.size(); i++) { + if (infoArgs[i]->events == arg->events) { + bool isSame = false; + napi_strict_equals(infoArgs[i]->env, infoArgs[i]->nRef.Deref(infoArgs[i]->env).val_, + arg->nRef.Deref(arg->env).val_, &isSame); + if (isSame) { + iter->second.erase(iter->second.begin() + i); + } + } + } + if (iter->second.size() == 0) { + watcherInfoMap_.erase(iter); + } +} + +int32_t FileWatcher::GetWatcherInfoSize(const int &wd) +{ + auto fileNameIter = wdFileNameMap_.find(wd); + if (fileNameIter == wdFileNameMap_.end()) { + return 0; + } + string fileName = fileNameIter->second; + auto watcherInfosIter = watcherInfoMap_.find(fileName); + if (watcherInfosIter == watcherInfoMap_.end()) { + return 0; + } + return watcherInfosIter->second.size(); +} + +bool CheckIncludeEvent(const uint32_t &allEvent, const uint32_t &event) +{ + vector events = GetEvents(event); + for (auto event : events) { + if (allEvent & event) { + return true; + } + } + return false; +} + +void FileWatcher::NotifyEvent(const struct inotify_event *event, WatcherCallback callback) +{ + lock_guard lock(watchMutex_); + auto fileNameIter = wdFileNameMap_.find(event->wd); + if (fileNameIter == wdFileNameMap_.end()) { + return; + } + + string fileName = fileNameIter->second; + auto watcherInfosIter = watcherInfoMap_.find(fileName); + if (watcherInfosIter == watcherInfoMap_.end()) { + return; + } + + auto args = watcherInfosIter->second; + for (auto arg : args) { + uint32_t watchEvent = arg->events; + if (!CheckIncludeEvent(event->mask, watchEvent)) { + continue; + } + if (event->len > 0) { + fileName.append(string("/")); + fileName.append(string(event->name)); + } + callback(arg->env, arg->nRef, fileName, event->mask, event->cookie); + } +} + +void FileWatcher::AddWatcherWdFileNameMap(const int &wd, const string &fileName) +{ + auto it = wdFileNameMap_.find(wd); + if (it == wdFileNameMap_.end()) { + wdFileNameMap_.insert(pair(wd, fileName)); + } +} + +void FileWatcher::RemoveWatcherWdFileNameMap(const int &wd) +{ + auto pathIter = wdFileNameMap_.find(wd); + if(pathIter != wdFileNameMap_.end()) { + string fileName = pathIter->second; + auto watcherInfosIter = watcherInfoMap_.find(fileName); + if (watcherInfosIter == watcherInfoMap_.end()) { + wdFileNameMap_.erase(pathIter); + } + } } } // namespace OHOS::FileManagement::ModuleFileIO diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index fad6ec27a..e7d6dab56 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -14,23 +14,30 @@ */ #ifndef INTERFACES_KITS_JS_SRC_MOD_FILEIO_CLASS_WATCHER_WATCHER_ENTITY_H #define INTERFACES_KITS_JS_SRC_MOD_FILEIO_CLASS_WATCHER_WATCHER_ENTITY_H + +#include #include +#include +#include #include #include +#include +#include #include #include "filemgmt_libn.h" + namespace OHOS::FileManagement::ModuleFileIO { using WatcherCallback = void (*)(napi_env env, LibN::NRef &callback, const std::string &filename, const uint32_t &event, const uint32_t &cookie); + constexpr int BUF_SIZE = 1024; struct WatcherInfoArg { - std::string filename = ""; + std::string fileName = ""; uint32_t events = 0; - int fd = -1; int wd = -1; napi_env env = nullptr; LibN::NRef nRef; @@ -38,24 +45,49 @@ struct WatcherInfoArg { ~WatcherInfoArg() = default; }; +using WatcherInfoMap = std::unordered_map>>; + class FileWatcher { public: - FileWatcher(); - ~FileWatcher(); - bool InitNotify(int &fd); - bool StartNotify(WatcherInfoArg &arg); - bool StopNotify(const WatcherInfoArg &arg); - void GetNotifyEvent(WatcherInfoArg &arg, WatcherCallback callback); + virtual ~FileWatcher() = default; + + static std::shared_ptr GetInstance(); + int32_t GetNotifyId(); + bool InitNotify(); + bool StartNotify(std::shared_ptr &arg); + bool StopNotify(const std::shared_ptr &arg); + void GetNotifyEvent(std::shared_ptr &arg, WatcherCallback callback); + void AddWatcherInfo(const std::string &fileName, const std::shared_ptr &arg); + +private: + FileWatcher() = default; + void AddWatcherWdFileNameMap(const int &wd, const std::string &fileName); + void RemoveWatcherWdFileNameMap(const int &wd); + void RemoveWatcherInfo(const std::shared_ptr &arg); + int32_t GetWatcherInfoSize(const int &wd); + bool CheckEventWatched(const std::string &fileName, const uint32_t &event); + std::vector ParseEvent(const std::string &fileName, const uint32_t &watcherEvent); + void NotifyEvent(const struct inotify_event *event, WatcherCallback callback); + bool NotifyToWatchNewEvents(const std::shared_ptr &arg); + bool FindWatcherInfo(const std::string &fileName, const uint32_t &event); + bool FindWatcherByFileName(const std::shared_ptr &arg); + void RemoveEventFromMap(const std::shared_ptr &arg); + bool CheckEventValid(const uint32_t &event); private: - void HandleEvent(WatcherInfoArg &arg, const struct inotify_event *event, - WatcherCallback callback); + static std::mutex mutex_; + static std::mutex watchMutex_; + static std::shared_ptr instance_; bool run_ = false; + int32_t notifyFd_ = -1; + WatcherInfoMap watcherInfoMap_; + std::map wdFileNameMap_; + std::map> fileNameEventMap_; + std::condition_variable stopWatcherCv_; }; struct WatcherEntity { - std::unique_ptr data_; - std::shared_ptr watcherPtr_; + std::shared_ptr data_; }; } // namespace OHOS::FileManagement::ModuleFileIO namespace OHOS #endif diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp index a1988cbeb..05be451ed 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp @@ -64,11 +64,10 @@ napi_value WatcherNExporter::Stop(napi_env env, napi_callback_info info) NError(EINVAL).ThrowErr(env); return nullptr; } - if (!watchEntity->watcherPtr_->StopNotify(*(watchEntity->data_))) { + if (!FileWatcher::GetInstance()->StopNotify(watchEntity->data_)) { NError(errno).ThrowErr(env); return nullptr; } - return NVal::CreateUndefined(env).val_; } @@ -86,13 +85,13 @@ napi_value WatcherNExporter::Start(napi_env env, napi_callback_info info) return nullptr; } - if (!watchEntity->watcherPtr_->StartNotify(*(watchEntity->data_))) { + if (!FileWatcher::GetInstance()->StartNotify(watchEntity->data_)) { NError(errno).ThrowErr(env); return nullptr; } auto cbExec = [watchEntity]() -> NError { - watchEntity->watcherPtr_->GetNotifyEvent(*(watchEntity->data_), WatcherCallback); + FileWatcher::GetInstance()->GetNotifyEvent(watchEntity->data_, WatcherCallback); return NError(ERRNO_NOERR); }; diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index 8d3fa0341..6fa89a6b5 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -31,7 +31,7 @@ using namespace std; using namespace OHOS::FileManagement::LibN; namespace { - const std::string STORAGE_DATA_PATH = "/data"; + const string STORAGE_DATA_PATH = "/data"; bool IsSystemApp() { uint64_t fullTokenId = OHOS::IPCSkeleton::GetCallingFullTokenID(); @@ -39,21 +39,16 @@ namespace { } } -static tuple, napi_value, NError> CreateAndCheckForWatcherEntity(napi_env env, int& fd) +static tuple CreateAndCheckForWatcherEntity(napi_env env, int& fd) { - auto watcherPtr = CreateSharedPtr(); - if (watcherPtr == nullptr) { - HILOGE("Failed to request heap memory."); - return {nullptr, nullptr, NError(ENOMEM)}; - } - if (!watcherPtr->InitNotify(fd)) { - return {watcherPtr, nullptr, NError(errno)}; + if (FileWatcher::GetInstance()->GetNotifyId() < 0 && !FileWatcher::GetInstance()->InitNotify()) { + return {nullptr, NError(errno)}; } napi_value objWatcher = NClass::InstantiateClass(env, WatcherNExporter::className_, {}); if (!objWatcher) { - return {watcherPtr, nullptr, NError(EINVAL)}; + return {nullptr, NError(EINVAL)}; } - return {watcherPtr, objWatcher, NError(ERRNO_NOERR)}; + return {objWatcher, NError(ERRNO_NOERR)}; } napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) @@ -75,15 +70,19 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) return nullptr; } - auto [succGetEvent, events] = NVal(env, funcArg[NARG_POS::SECOND]).ToUint32(); + auto [succGetEvent, events] = NVal(env, funcArg[NARG_POS::SECOND]).ToInt32(); if (!succGetEvent) { NError(EINVAL).ThrowErr(env); return nullptr; } + if (events <= 0) { + NError(EINVAL).ThrowErr(env); + return nullptr; + } int fd = -1; - auto [watcherPtr, objWatcher, err] = CreateAndCheckForWatcherEntity(env, fd); - if (watcherPtr == nullptr || !objWatcher) { + auto [objWatcher, err] = CreateAndCheckForWatcherEntity(env, fd); + if (!objWatcher) { err.ThrowErr(env); return nullptr; } @@ -98,18 +97,17 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) NError(EINVAL).ThrowErr(env); return nullptr; } - watcherEntity->data_ = CreateUniquePtr(NVal(env, funcArg[NARG_POS::THIRD])); - if (watcherEntity->data_ == nullptr) { + auto infoArg = CreateSharedPtr(NVal(env, funcArg[NARG_POS::THIRD])); + if (infoArg == nullptr) { HILOGE("Failed to request heap memory."); NError(ENOMEM).ThrowErr(env); return nullptr; } - watcherEntity->data_->events = events; - watcherEntity->data_->env = env; - watcherEntity->data_->filename = string(filename.get()); - watcherEntity->data_->fd = fd; - - watcherEntity->watcherPtr_ = watcherPtr; + infoArg->events = events; + infoArg->env = env; + infoArg->fileName = string(filename.get()); + watcherEntity->data_ = infoArg; + FileWatcher::GetInstance()->AddWatcherInfo(string(filename.get()), infoArg); return objWatcher; } -- Gitee From 80e96f07c75dbe7f6adc86924803bbde3e26b029 Mon Sep 17 00:00:00 2001 From: Cao Chuan Date: Tue, 20 Jun 2023 16:08:38 +0800 Subject: [PATCH 02/12] modify code review Signed-off-by: Cao Chuan --- .../kits/js/src/mod_fs/class_watcher/watcher_entity.cpp | 4 ++-- interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h | 2 +- .../kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp | 2 +- interfaces/kits/js/src/mod_fs/properties/watcher.cpp | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index d629465ea..9ff4509b9 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -151,7 +151,7 @@ bool FileWatcher::StartNotify(shared_ptr &arg) bool FileWatcher::FindWatcherInfo(const std::string &fileName, const uint32_t &event) { - auto iter = watcherInfoMap_.find(fileName); + auto iter = watcherInfoMap_.find(fileName); if (iter == watcherInfoMap_.end()) { return false; } @@ -287,7 +287,7 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) return true; } -void FileWatcher::GetNotifyEvent(shared_ptr &arg, WatcherCallback callback) +void FileWatcher::GetNotifyEvent(WatcherCallback callback) { if (run_) { return; diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index e7d6dab56..36fb462cd 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -56,7 +56,7 @@ public: bool InitNotify(); bool StartNotify(std::shared_ptr &arg); bool StopNotify(const std::shared_ptr &arg); - void GetNotifyEvent(std::shared_ptr &arg, WatcherCallback callback); + void GetNotifyEvent(WatcherCallback callback); void AddWatcherInfo(const std::string &fileName, const std::shared_ptr &arg); private: diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp index 05be451ed..6dba5e0b6 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp @@ -91,7 +91,7 @@ napi_value WatcherNExporter::Start(napi_env env, napi_callback_info info) } auto cbExec = [watchEntity]() -> NError { - FileWatcher::GetInstance()->GetNotifyEvent(watchEntity->data_, WatcherCallback); + FileWatcher::GetInstance()->GetNotifyEvent(WatcherCallback); return NError(ERRNO_NOERR); }; diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index 6fa89a6b5..2fa996f22 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -39,7 +39,7 @@ namespace { } } -static tuple CreateAndCheckForWatcherEntity(napi_env env, int& fd) +static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) { if (FileWatcher::GetInstance()->GetNotifyId() < 0 && !FileWatcher::GetInstance()->InitNotify()) { return {nullptr, NError(errno)}; @@ -108,7 +108,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) infoArg->fileName = string(filename.get()); watcherEntity->data_ = infoArg; FileWatcher::GetInstance()->AddWatcherInfo(string(filename.get()), infoArg); - + return objWatcher; } } // namespace OHOS::FileManagement::ModuleFileIO -- Gitee From 1854b11df87c959f4bffd0f509f6a53cad2436c8 Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Mon, 26 Jun 2023 17:17:06 +0800 Subject: [PATCH 03/12] modify code review Signed-off-by: CaoChuan --- .../mod_fs/class_watcher/watcher_entity.cpp | 46 +++++++++---------- .../src/mod_fs/class_watcher/watcher_entity.h | 4 +- .../kits/js/src/mod_fs/properties/watcher.cpp | 23 +++++----- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index 9ff4509b9..d9ea7c032 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -29,7 +29,6 @@ using namespace OHOS::FileManagement::LibN; using namespace std; namespace { - constexpr int64_t SELECT_TIME_OUT_US = 100000; constexpr int64_t STOP_WATCHER_TIME_OUT_US = 120000; const vector EVENT_LIST = { IN_ACCESS, IN_MODIFY, IN_ATTRIB, IN_CLOSE_WRITE, IN_CLOSE_NOWRITE, IN_OPEN, IN_MOVED_FROM, IN_MOVED_TO, IN_CREATE, IN_DELETE, @@ -69,7 +68,7 @@ bool FileWatcher::InitNotify() vector GetEvents(const uint32_t &watchEvent) { vector eventList; - for (auto event : EVENT_LIST) { + for (const auto &event : EVENT_LIST) { if (watchEvent & event) { eventList.push_back(event); } @@ -81,7 +80,7 @@ vector FindUnWatchedEvent(const vector &allEvents, const uin { vector unWatchedEvents; vector watchEvents = GetEvents(event); - for (auto event: watchEvents) { + for (const auto &event : watchEvents) { auto iter = find(allEvents.begin(), allEvents.end(), event); if (iter == allEvents.end()) { unWatchedEvents.push_back(event); @@ -136,7 +135,7 @@ bool FileWatcher::StartNotify(shared_ptr &arg) } vector watchEvents = ParseEvent(arg->fileName.c_str(), arg->events); uint32_t watchEvent = 0; - for (auto event: watchEvents) { + for (const auto &event : watchEvents) { watchEvent |= event; } int wd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); @@ -156,7 +155,7 @@ bool FileWatcher::FindWatcherInfo(const std::string &fileName, const uint32_t &e return false; } vector> args = iter->second; - for (auto arg : args) { + for (const auto &arg : args) { if (arg->events == event) { return true; } @@ -172,7 +171,7 @@ void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) } auto args = watcherInfosIter->second; vector watchedEvent; - for (auto arg : args) { + for (const auto &arg : args) { uint32_t event = arg->events; vector events = GetEvents(event); watchedEvent.insert(watchedEvent.end(), events.begin(), events.end()); @@ -183,7 +182,7 @@ void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) map>::iterator fileIter; uint32_t removeEvent = arg->events; vector removeEvents = GetEvents(removeEvent); - for (auto toRemoveEvent : removeEvents) { + for (const auto &toRemoveEvent : removeEvents) { auto watchedIter = find(watchedEvent.begin(), watchedEvent.end(), toRemoveEvent); if (watchedIter != watchedEvent.end()) { continue; @@ -216,11 +215,10 @@ bool FileWatcher::NotifyToWatchNewEvents(const shared_ptr &arg) } vector watchEvents = eventIter->second; uint32_t watchEvent = 0; - for (auto event : watchEvents) { + for (const auto &event : watchEvents) { watchEvent |= event; } int newWd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); - if (newWd < 0) { HILOGE("Failed to start new notify fail errCode:%{public}d", errno); return false; @@ -293,19 +291,19 @@ void FileWatcher::GetNotifyEvent(WatcherCallback callback) return; } - run_ =true; + run_ = true; char buf[BUF_SIZE] = {0}; struct inotify_event *event = nullptr; + fd_set fds; + FD_ZERO(&fds); + FD_SET(notifyFd_, &fds); while (run_) { if (notifyFd_ < 0) { HILOGE("Failed to run Listener Thread because notifyFd_:%{public}d", notifyFd_); break; } - fd_set fds; - FD_ZERO(&fds); - FD_SET(notifyFd_, &fds); - timeval timeout{.tv_sec = 0, .tv_usec = SELECT_TIME_OUT_US}; - if (select(notifyFd_ + 1, &fds, nullptr, nullptr, &timeout) > 0) { + + if (select(notifyFd_ + 1, &fds, nullptr, nullptr, 0) > 0) { int len, index = 0; while (((len = read(notifyFd_, &buf, sizeof(buf))) < 0) && (errno == EINTR)) {}; while (index < len) { @@ -321,24 +319,26 @@ void FileWatcher::GetNotifyEvent(WatcherCallback callback) } } -void FileWatcher::AddWatcherInfo(const string &fileName, const shared_ptr &arg) +bool FileWatcher::AddWatcherInfo(const string &fileName, const shared_ptr &arg) { WatcherInfoMap::iterator it = watcherInfoMap_.find(fileName); if (it == watcherInfoMap_.end()) { vector> watcherInfoVec = { arg }; watcherInfoMap_[fileName] = watcherInfoVec; - return; + return true; } auto watcherInfos = it->second; - for (auto &info : watcherInfos) { + for (const auto &info : watcherInfos) { bool isSame = false; napi_strict_equals(info->env, info->nRef.Deref(info->env).val_, arg->nRef.Deref(arg->env).val_, &isSame); if (isSame && info->events == arg->events) { - return; + HILOGE("Faile to add watcher, fileName:%{public}s the callback is same", fileName.c_str()); + return false; } } it->second.push_back(arg); + return true; } vector FileWatcher::ParseEvent(const string &fileName, const uint32_t &watcherEvent) @@ -356,7 +356,7 @@ vector FileWatcher::ParseEvent(const string &fileName, const uint32_t newEvents.insert(newEvents.end(), notWatchedEvents.begin(), notWatchedEvents.end()); sort(newEvents.begin(), newEvents.end()); auto newEventIter = unique(newEvents.begin(), newEvents.end()); - newEvents.erase(newEventIter, newEvents.end()); + newEvents.erase(newEventIter, newEvents.end()); fileNameEventMap_[fileName] = newEvents; return newEvents; } else { @@ -409,7 +409,7 @@ int32_t FileWatcher::GetWatcherInfoSize(const int &wd) bool CheckIncludeEvent(const uint32_t &allEvent, const uint32_t &event) { vector events = GetEvents(event); - for (auto event : events) { + for (const auto &event : events) { if (allEvent & event) { return true; } @@ -432,7 +432,7 @@ void FileWatcher::NotifyEvent(const struct inotify_event *event, WatcherCallback } auto args = watcherInfosIter->second; - for (auto arg : args) { + for (const auto &arg : args) { uint32_t watchEvent = arg->events; if (!CheckIncludeEvent(event->mask, watchEvent)) { continue; @@ -456,7 +456,7 @@ void FileWatcher::AddWatcherWdFileNameMap(const int &wd, const string &fileName) void FileWatcher::RemoveWatcherWdFileNameMap(const int &wd) { auto pathIter = wdFileNameMap_.find(wd); - if(pathIter != wdFileNameMap_.end()) { + if (pathIter != wdFileNameMap_.end()) { string fileName = pathIter->second; auto watcherInfosIter = watcherInfoMap_.find(fileName); if (watcherInfosIter == watcherInfoMap_.end()) { diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index 36fb462cd..69a1cfe7d 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -57,7 +57,8 @@ public: bool StartNotify(std::shared_ptr &arg); bool StopNotify(const std::shared_ptr &arg); void GetNotifyEvent(WatcherCallback callback); - void AddWatcherInfo(const std::string &fileName, const std::shared_ptr &arg); + bool AddWatcherInfo(const std::string &fileName, const std::shared_ptr &arg); + bool CheckEventValid(const uint32_t &event); private: FileWatcher() = default; @@ -72,7 +73,6 @@ private: bool FindWatcherInfo(const std::string &fileName, const uint32_t &event); bool FindWatcherByFileName(const std::shared_ptr &arg); void RemoveEventFromMap(const std::shared_ptr &arg); - bool CheckEventValid(const uint32_t &event); private: static std::mutex mutex_; diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index 2fa996f22..13a4081ec 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -31,7 +31,6 @@ using namespace std; using namespace OHOS::FileManagement::LibN; namespace { - const string STORAGE_DATA_PATH = "/data"; bool IsSystemApp() { uint64_t fullTokenId = OHOS::IPCSkeleton::GetCallingFullTokenID(); @@ -39,16 +38,16 @@ namespace { } } -static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) +static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) { if (FileWatcher::GetInstance()->GetNotifyId() < 0 && !FileWatcher::GetInstance()->InitNotify()) { - return {nullptr, NError(errno)}; + return {nullptr, errno}; } napi_value objWatcher = NClass::InstantiateClass(env, WatcherNExporter::className_, {}); if (!objWatcher) { - return {nullptr, NError(EINVAL)}; + return {nullptr, EIO}; } - return {objWatcher, NError(ERRNO_NOERR)}; + return {objWatcher, ERRNO_NOERR}; } napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) @@ -71,11 +70,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) } auto [succGetEvent, events] = NVal(env, funcArg[NARG_POS::SECOND]).ToInt32(); - if (!succGetEvent) { - NError(EINVAL).ThrowErr(env); - return nullptr; - } - if (events <= 0) { + if (!succGetEvent || events <= 0 || !FileWatcher::GetInstance()->CheckEventValid(events)) { NError(EINVAL).ThrowErr(env); return nullptr; } @@ -83,7 +78,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) int fd = -1; auto [objWatcher, err] = CreateAndCheckForWatcherEntity(env, fd); if (!objWatcher) { - err.ThrowErr(env); + NError(err).ThrowErr(env); return nullptr; } @@ -107,7 +102,11 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) infoArg->env = env; infoArg->fileName = string(filename.get()); watcherEntity->data_ = infoArg; - FileWatcher::GetInstance()->AddWatcherInfo(string(filename.get()), infoArg); + bool ret = FileWatcher::GetInstance()->AddWatcherInfo(string(filename.get()), infoArg); + if (!ret) { + NError(UNKROWN_ERR).ThrowErr(env); + return nullptr; + } return objWatcher; } -- Gitee From a632b7ff7f9d7caec112cc019f51b8478368c8fe Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Tue, 27 Jun 2023 13:15:16 +0800 Subject: [PATCH 04/12] modify single Signed-off-by: CaoChuan --- .../js/src/mod_fs/class_watcher/watcher_entity.cpp | 14 +++++--------- .../js/src/mod_fs/class_watcher/watcher_entity.h | 12 +++++++----- .../mod_fs/class_watcher/watcher_n_exporter.cpp | 6 +++--- .../kits/js/src/mod_fs/properties/watcher.cpp | 6 +++--- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index d9ea7c032..60ca0c2e0 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -35,19 +35,15 @@ namespace { IN_DELETE_SELF, IN_MOVE_SELF }; } -shared_ptr FileWatcher::instance_ = nullptr; mutex FileWatcher::mutex_; mutex FileWatcher::watchMutex_; -shared_ptr FileWatcher::GetInstance() +FileWatcher::FileWatcher() +{ +} + +FileWatcher::~FileWatcher() { - if (instance_ == nullptr) { - lock_guard lock(mutex_); - if (instance_ == nullptr) { - instance_ = shared_ptr(new FileWatcher()); - } - } - return instance_; } int32_t FileWatcher::GetNotifyId() diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index 69a1cfe7d..f11fe31af 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -26,7 +26,7 @@ #include #include "filemgmt_libn.h" - +#include "singleton.h" namespace OHOS::FileManagement::ModuleFileIO { using WatcherCallback = void (*)(napi_env env, LibN::NRef &callback, @@ -47,11 +47,14 @@ struct WatcherInfoArg { using WatcherInfoMap = std::unordered_map>>; -class FileWatcher { +class FileWatcher : public Singleton { public: - virtual ~FileWatcher() = default; + FileWatcher(); + ~FileWatcher(); + + FileWatcher(FileWatcher const &) = delete; + void operator=(FileWatcher const &) = delete; - static std::shared_ptr GetInstance(); int32_t GetNotifyId(); bool InitNotify(); bool StartNotify(std::shared_ptr &arg); @@ -61,7 +64,6 @@ public: bool CheckEventValid(const uint32_t &event); private: - FileWatcher() = default; void AddWatcherWdFileNameMap(const int &wd, const std::string &fileName); void RemoveWatcherWdFileNameMap(const int &wd); void RemoveWatcherInfo(const std::shared_ptr &arg); diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp index 6dba5e0b6..a784eab62 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp @@ -64,7 +64,7 @@ napi_value WatcherNExporter::Stop(napi_env env, napi_callback_info info) NError(EINVAL).ThrowErr(env); return nullptr; } - if (!FileWatcher::GetInstance()->StopNotify(watchEntity->data_)) { + if (!FileWatcher::GetInstance().StopNotify(watchEntity->data_)) { NError(errno).ThrowErr(env); return nullptr; } @@ -85,13 +85,13 @@ napi_value WatcherNExporter::Start(napi_env env, napi_callback_info info) return nullptr; } - if (!FileWatcher::GetInstance()->StartNotify(watchEntity->data_)) { + if (!FileWatcher::GetInstance().StartNotify(watchEntity->data_)) { NError(errno).ThrowErr(env); return nullptr; } auto cbExec = [watchEntity]() -> NError { - FileWatcher::GetInstance()->GetNotifyEvent(WatcherCallback); + FileWatcher::GetInstance().GetNotifyEvent(WatcherCallback); return NError(ERRNO_NOERR); }; diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index 13a4081ec..7e56a83c3 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -40,7 +40,7 @@ namespace { static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) { - if (FileWatcher::GetInstance()->GetNotifyId() < 0 && !FileWatcher::GetInstance()->InitNotify()) { + if (FileWatcher::GetInstance().GetNotifyId() < 0 && !FileWatcher::GetInstance().InitNotify()) { return {nullptr, errno}; } napi_value objWatcher = NClass::InstantiateClass(env, WatcherNExporter::className_, {}); @@ -70,7 +70,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) } auto [succGetEvent, events] = NVal(env, funcArg[NARG_POS::SECOND]).ToInt32(); - if (!succGetEvent || events <= 0 || !FileWatcher::GetInstance()->CheckEventValid(events)) { + if (!succGetEvent || events <= 0 || !FileWatcher::GetInstance().CheckEventValid(events)) { NError(EINVAL).ThrowErr(env); return nullptr; } @@ -102,7 +102,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) infoArg->env = env; infoArg->fileName = string(filename.get()); watcherEntity->data_ = infoArg; - bool ret = FileWatcher::GetInstance()->AddWatcherInfo(string(filename.get()), infoArg); + bool ret = FileWatcher::GetInstance().AddWatcherInfo(string(filename.get()), infoArg); if (!ret) { NError(UNKROWN_ERR).ThrowErr(env); return nullptr; -- Gitee From 82e79a0ff900a43dcd018e02f80a0bc4eef295a4 Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Tue, 27 Jun 2023 14:18:53 +0800 Subject: [PATCH 05/12] detail bug Signed-off-by: CaoChuan --- .../kits/js/src/mod_fs/class_watcher/watcher_entity.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index 60ca0c2e0..a8085a463 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -421,14 +421,15 @@ void FileWatcher::NotifyEvent(const struct inotify_event *event, WatcherCallback return; } - string fileName = fileNameIter->second; - auto watcherInfosIter = watcherInfoMap_.find(fileName); + string tempFileName = fileNameIter->second; + auto watcherInfosIter = watcherInfoMap_.find(tempFileName); if (watcherInfosIter == watcherInfoMap_.end()) { return; } auto args = watcherInfosIter->second; for (const auto &arg : args) { + string fileName = tempFileName; uint32_t watchEvent = arg->events; if (!CheckIncludeEvent(event->mask, watchEvent)) { continue; -- Gitee From a19b897cd38b39522d8ad5960ee0049c71b7383e Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Wed, 28 Jun 2023 09:46:47 +0800 Subject: [PATCH 06/12] modify over 50 lines Signed-off-by: CaoChuan --- .../class_watcher/watcher_n_exporter.cpp | 2 +- .../kits/js/src/mod_fs/properties/watcher.cpp | 64 +++++++++++-------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp index a784eab62..acf7e2fca 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp @@ -168,7 +168,7 @@ void WatcherNExporter::WatcherCallback(napi_env env, NRef &callback, const std:: } if (!callback) { - HILOGE("Failed to pass watcher callback"); + HILOGE("Failed to parse watcher callback"); return; } diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index 7e56a83c3..b3fc4adf0 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -50,28 +50,56 @@ static tuple CreateAndCheckForWatcherEntity(napi_env env, i return {objWatcher, ERRNO_NOERR}; } -napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) +shared_ptr ParseParam(const napi_env env, const napi_callback_info info, int32_t &errCode) { - if (!IsSystemApp()) { - NError(E_PERMISSION_SYS).ThrowErr(env); - return nullptr; - } - NFuncArg funcArg(env, info); if (!funcArg.InitArgs(NARG_CNT::THREE)) { - NError(EINVAL).ThrowErr(env); + errCode = EINVAL; return nullptr; } auto [succGetPath, filename, unused] = NVal(env, funcArg[NARG_POS::FIRST]).ToUTF8String(); if (!succGetPath) { - NError(EINVAL).ThrowErr(env); + HILOGE("Failed to get watcher path."); + errCode = EINVAL; return nullptr; } auto [succGetEvent, events] = NVal(env, funcArg[NARG_POS::SECOND]).ToInt32(); if (!succGetEvent || events <= 0 || !FileWatcher::GetInstance().CheckEventValid(events)) { - NError(EINVAL).ThrowErr(env); + HILOGE("Failed to get watcher event."); + errCode = EINVAL; + return nullptr; + } + + if (!NVal(env, funcArg[NARG_POS::THIRD]).TypeIs(napi_function)) { + errCode = EINVAL; + return nullptr; + } + auto infoArg = CreateSharedPtr(NVal(env, funcArg[NARG_POS::THIRD])); + if (infoArg == nullptr) { + HILOGE("Failed to request heap memory."); + errCode = ENOMEM; + return nullptr; + } + infoArg->events = events; + infoArg->env = env; + infoArg->fileName = string(filename.get()); + + return infoArg; +} + +napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) +{ + if (!IsSystemApp()) { + NError(E_PERMISSION_SYS).ThrowErr(env); + return nullptr; + } + + int errCode = -1; + auto infoArg = ParseParam(env, info, errCode); + if (infoArg == nullptr) { + NError(errCode).ThrowErr(env); return nullptr; } @@ -87,27 +115,13 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) NError(EINVAL).ThrowErr(env); return nullptr; } - - if (!NVal(env, funcArg[NARG_POS::THIRD]).TypeIs(napi_function)) { - NError(EINVAL).ThrowErr(env); - return nullptr; - } - auto infoArg = CreateSharedPtr(NVal(env, funcArg[NARG_POS::THIRD])); - if (infoArg == nullptr) { - HILOGE("Failed to request heap memory."); - NError(ENOMEM).ThrowErr(env); - return nullptr; - } - infoArg->events = events; - infoArg->env = env; - infoArg->fileName = string(filename.get()); watcherEntity->data_ = infoArg; - bool ret = FileWatcher::GetInstance().AddWatcherInfo(string(filename.get()), infoArg); + + bool ret = FileWatcher::GetInstance().AddWatcherInfo(infoArg->fileName, infoArg); if (!ret) { NError(UNKROWN_ERR).ThrowErr(env); return nullptr; } - return objWatcher; } } // namespace OHOS::FileManagement::ModuleFileIO -- Gitee From 32f0e5de8012898e3f753fe4326fd69f1bb53c66 Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Wed, 28 Jun 2023 14:52:19 +0800 Subject: [PATCH 07/12] remove unused Signed-off-by: CaoChuan --- interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp | 1 - interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index a8085a463..ed2ef80fb 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -35,7 +35,6 @@ namespace { IN_DELETE_SELF, IN_MOVE_SELF }; } -mutex FileWatcher::mutex_; mutex FileWatcher::watchMutex_; FileWatcher::FileWatcher() diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index f11fe31af..4af342d95 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -77,9 +77,7 @@ private: void RemoveEventFromMap(const std::shared_ptr &arg); private: - static std::mutex mutex_; static std::mutex watchMutex_; - static std::shared_ptr instance_; bool run_ = false; int32_t notifyFd_ = -1; WatcherInfoMap watcherInfoMap_; -- Gitee From eaef4aa98df19cd29a7e391a6347951df657722b Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Wed, 28 Jun 2023 17:24:04 +0800 Subject: [PATCH 08/12] add close log Signed-off-by: CaoChuan --- .../kits/js/src/mod_fs/class_watcher/watcher_entity.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index ed2ef80fb..ad1f43487 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -270,7 +270,10 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) } RemoveWatcherWdFileNameMap(arg->wd); if (watcherInfoMap_.size() == 0) { - close(notifyFd_); + int closeRet = close(notifyFd_); + if (closeRet != 0) { + HILOGE("Failed to stop notify close fd errCode:%{public}d", closeRet); + } notifyFd_ = -1; run_ = false; stopWatcherCv_.wait_for(lock, chrono::milliseconds(STOP_WATCHER_TIME_OUT_US)); -- Gitee From 51f5668e64536b5340401f93604a82f16621f6eb Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Mon, 3 Jul 2023 16:38:35 +0800 Subject: [PATCH 09/12] drop stopWatcherCv_ Signed-off-by: CaoChuan --- .../kits/js/src/mod_fs/class_watcher/watcher_entity.cpp | 6 ------ .../kits/js/src/mod_fs/class_watcher/watcher_entity.h | 1 - interfaces/kits/js/src/mod_fs/properties/watcher.cpp | 2 +- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index ad1f43487..4e54eef76 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -29,7 +29,6 @@ using namespace OHOS::FileManagement::LibN; using namespace std; namespace { - constexpr int64_t STOP_WATCHER_TIME_OUT_US = 120000; const vector EVENT_LIST = { IN_ACCESS, IN_MODIFY, IN_ATTRIB, IN_CLOSE_WRITE, IN_CLOSE_NOWRITE, IN_OPEN, IN_MOVED_FROM, IN_MOVED_TO, IN_CREATE, IN_DELETE, IN_DELETE_SELF, IN_MOVE_SELF }; @@ -276,7 +275,6 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) } notifyFd_ = -1; run_ = false; - stopWatcherCv_.wait_for(lock, chrono::milliseconds(STOP_WATCHER_TIME_OUT_US)); } return true; } @@ -311,10 +309,6 @@ void FileWatcher::GetNotifyEvent(WatcherCallback callback) } } } - - if (!run_) { - stopWatcherCv_.notify_one(); - } } bool FileWatcher::AddWatcherInfo(const string &fileName, const shared_ptr &arg) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index 4af342d95..3ce489a4b 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -83,7 +83,6 @@ private: WatcherInfoMap watcherInfoMap_; std::map wdFileNameMap_; std::map> fileNameEventMap_; - std::condition_variable stopWatcherCv_; }; struct WatcherEntity { diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index b3fc4adf0..efeb0be64 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -119,7 +119,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) bool ret = FileWatcher::GetInstance().AddWatcherInfo(infoArg->fileName, infoArg); if (!ret) { - NError(UNKROWN_ERR).ThrowErr(env); + NError(EINVAL).ThrowErr(env); return nullptr; } return objWatcher; -- Gitee From 945467dae8b31b63aa2a13c4503d85e3cdd734cf Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Tue, 4 Jul 2023 11:04:38 +0800 Subject: [PATCH 10/12] modify header Signed-off-by: CaoChuan --- interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index 3ce489a4b..6ab6d5253 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -12,8 +12,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#ifndef INTERFACES_KITS_JS_SRC_MOD_FILEIO_CLASS_WATCHER_WATCHER_ENTITY_H -#define INTERFACES_KITS_JS_SRC_MOD_FILEIO_CLASS_WATCHER_WATCHER_ENTITY_H +#ifndef INTERFACES_KITS_JS_SRC_MOD_FS_CLASS_WATCHER_WATCHER_ENTITY_H +#define INTERFACES_KITS_JS_SRC_MOD_FS_CLASS_WATCHER_WATCHER_ENTITY_H #include #include -- Gitee From 035049aeb39e3816c28a626faf10a00ada64cb90 Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Thu, 6 Jul 2023 16:04:27 +0800 Subject: [PATCH 11/12] add error log Signed-off-by: CaoChuan --- .../js/src/mod_fs/class_watcher/watcher_entity.cpp | 11 +++++++++-- .../kits/js/src/mod_fs/class_watcher/watcher_entity.h | 2 +- .../src/mod_fs/class_watcher/watcher_n_exporter.cpp | 11 +++++++++++ interfaces/kits/js/src/mod_fs/properties/watcher.cpp | 9 +++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index 4e54eef76..cbfe68e03 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -125,6 +125,7 @@ bool FileWatcher::StartNotify(shared_ptr &arg) return true; } } + HILOGE("Failed to start notify, not find fileName:%{public}s", arg->fileName.c_str()); return false; } vector watchEvents = ParseEvent(arg->fileName.c_str(), arg->events); @@ -142,7 +143,7 @@ bool FileWatcher::StartNotify(shared_ptr &arg) return true; } -bool FileWatcher::FindWatcherInfo(const std::string &fileName, const uint32_t &event) +bool FileWatcher::CheckFileWatched(const std::string &fileName, const uint32_t &event) { auto iter = watcherInfoMap_.find(fileName); if (iter == watcherInfoMap_.end()) { @@ -161,6 +162,7 @@ void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) { auto watcherInfosIter = watcherInfoMap_.find(arg->fileName); if (watcherInfosIter == watcherInfoMap_.end()) { + HILOGE("Failed to remove filename:%{public}s from map, it is not exists in the map", arg->fileName.c_str()); return; } auto args = watcherInfosIter->second; @@ -199,7 +201,7 @@ void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) bool FileWatcher::NotifyToWatchNewEvents(const shared_ptr &arg) { - if (FindWatcherInfo(arg->fileName, arg->events)) { + if (CheckFileWatched(arg->fileName, arg->events)) { return true; } RemoveEventFromMap(arg); @@ -246,6 +248,7 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) if (FindWatcherByFileName(arg)) { bool ret = NotifyToWatchNewEvents(arg); if (!ret) { + HILOGE("Failed to stop notify combination new events error"); return false; } } else { @@ -258,6 +261,7 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) if (inotify_rm_watch(notifyFd_, arg->wd) == -1) { auto iter = wdFileNameMap_.find(arg->wd); if (iter == wdFileNameMap_.end()) { + HILOGE("Failed to stop notify not find wd:%{public}d", arg->wd); return false; } string filename = iter->second; @@ -361,11 +365,13 @@ void FileWatcher::RemoveWatcherInfo(const shared_ptr &arg) { auto fileNameIter = wdFileNameMap_.find(arg->wd); if (fileNameIter == wdFileNameMap_.end()) { + HILOGE("Failed to remove wd:%{public}d from map, it is not exist in the map", arg->wd); return; } string fileName = fileNameIter->second; auto iter = watcherInfoMap_.find(fileName); if (iter == watcherInfoMap_.end()) { + HILOGE("Failed to remove fileName:%{public}s from map, it is not exist in the map", fileName.c_str()); return; } auto infoArgs = iter->second; @@ -420,6 +426,7 @@ void FileWatcher::NotifyEvent(const struct inotify_event *event, WatcherCallback string tempFileName = fileNameIter->second; auto watcherInfosIter = watcherInfoMap_.find(tempFileName); if (watcherInfosIter == watcherInfoMap_.end()) { + HILOGE("Not find the callback of filename:%{public}s", tempFileName.c_str()); return; } diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index 6ab6d5253..cad9926b5 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -72,7 +72,7 @@ private: std::vector ParseEvent(const std::string &fileName, const uint32_t &watcherEvent); void NotifyEvent(const struct inotify_event *event, WatcherCallback callback); bool NotifyToWatchNewEvents(const std::shared_ptr &arg); - bool FindWatcherInfo(const std::string &fileName, const uint32_t &event); + bool CheckFileWatched(const std::string &fileName, const uint32_t &event); bool FindWatcherByFileName(const std::shared_ptr &arg); void RemoveEventFromMap(const std::shared_ptr &arg); diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp index acf7e2fca..188d28594 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_n_exporter.cpp @@ -34,6 +34,7 @@ napi_value WatcherNExporter::Constructor(napi_env env, napi_callback_info info) { NFuncArg funcArg(env, info); if (!funcArg.InitArgs(NARG_CNT::ZERO)) { + HILOGE("Failed to get param."); NError(EINVAL).ThrowErr(env); return nullptr; } @@ -45,6 +46,7 @@ napi_value WatcherNExporter::Constructor(napi_env env, napi_callback_info info) return nullptr; } if (!NClass::SetEntityFor(env, funcArg.GetThisVar(), move(watcherEntity))) { + HILOGE("Failed to set watcherEntity."); NError(EIO).ThrowErr(env); return nullptr; } @@ -55,16 +57,19 @@ napi_value WatcherNExporter::Stop(napi_env env, napi_callback_info info) { NFuncArg funcArg(env, info); if (!funcArg.InitArgs(NARG_CNT::ZERO)) { + HILOGE("Failed to get param when stop."); NError(EINVAL).ThrowErr(env); return nullptr; } auto watchEntity = NClass::GetEntityOf(env, funcArg.GetThisVar()); if (!watchEntity) { + HILOGE("Failed to get watcherEntity when stop."); NError(EINVAL).ThrowErr(env); return nullptr; } if (!FileWatcher::GetInstance().StopNotify(watchEntity->data_)) { + HILOGE("Failed to stopNotify."); NError(errno).ThrowErr(env); return nullptr; } @@ -75,17 +80,20 @@ napi_value WatcherNExporter::Start(napi_env env, napi_callback_info info) { NFuncArg funcArg(env, info); if (!funcArg.InitArgs(NARG_CNT::ZERO)) { + HILOGE("Failed to get param when start."); NError(EINVAL).ThrowErr(env); return nullptr; } auto watchEntity = NClass::GetEntityOf(env, funcArg.GetThisVar()); if (!watchEntity) { + HILOGE("Failed to get watcherEntity when start."); NError(EINVAL).ThrowErr(env); return nullptr; } if (!FileWatcher::GetInstance().StartNotify(watchEntity->data_)) { + HILOGE("Failed to startNotify."); NError(errno).ThrowErr(env); return nullptr; } @@ -97,6 +105,7 @@ napi_value WatcherNExporter::Start(napi_env env, napi_callback_info info) auto cbCompl = [](napi_env env, NError err) -> NVal { if (err) { + HILOGE("Failed to execute complete."); return {env, err.GetNapiErr(env)}; } return {NVal::CreateUndefined(env)}; @@ -199,12 +208,14 @@ bool WatcherNExporter::Export() auto [resDefineClass, classValue] = NClass::DefineClass(exports_.env_, className, WatcherNExporter::Constructor, std::move(props)); if (!resDefineClass) { + HILOGE("Failed to DefineClass"); NError(EIO).ThrowErr(exports_.env_); return false; } bool succ = NClass::SaveClass(exports_.env_, className, classValue); if (!succ) { + HILOGE("Failed to SaveClass"); NError(EIO).ThrowErr(exports_.env_); return false; } diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index efeb0be64..d351eec07 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -41,10 +41,12 @@ namespace { static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) { if (FileWatcher::GetInstance().GetNotifyId() < 0 && !FileWatcher::GetInstance().InitNotify()) { + HILOGE("Failed to get notifyId Or initnotify fail"); return {nullptr, errno}; } napi_value objWatcher = NClass::InstantiateClass(env, WatcherNExporter::className_, {}); if (!objWatcher) { + HILOGE("Failed to instantiate watcher"); return {nullptr, EIO}; } return {objWatcher, ERRNO_NOERR}; @@ -54,6 +56,7 @@ shared_ptr ParseParam(const napi_env env, const napi_callback_in { NFuncArg funcArg(env, info); if (!funcArg.InitArgs(NARG_CNT::THREE)) { + HILOGE("Failed to get param."); errCode = EINVAL; return nullptr; } @@ -73,6 +76,7 @@ shared_ptr ParseParam(const napi_env env, const napi_callback_in } if (!NVal(env, funcArg[NARG_POS::THIRD]).TypeIs(napi_function)) { + HILOGE("Failed to get callback"); errCode = EINVAL; return nullptr; } @@ -92,6 +96,7 @@ shared_ptr ParseParam(const napi_env env, const napi_callback_in napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) { if (!IsSystemApp()) { + HILOGE("The hap is not system app."); NError(E_PERMISSION_SYS).ThrowErr(env); return nullptr; } @@ -99,6 +104,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) int errCode = -1; auto infoArg = ParseParam(env, info, errCode); if (infoArg == nullptr) { + HILOGE("Failed to parse param"); NError(errCode).ThrowErr(env); return nullptr; } @@ -106,12 +112,14 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) int fd = -1; auto [objWatcher, err] = CreateAndCheckForWatcherEntity(env, fd); if (!objWatcher) { + HILOGE("Failed to create watcher entity."); NError(err).ThrowErr(env); return nullptr; } auto watcherEntity = NClass::GetEntityOf(env, objWatcher); if (!watcherEntity) { + HILOGE("Failed to get WatcherEntity."); NError(EINVAL).ThrowErr(env); return nullptr; } @@ -119,6 +127,7 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) bool ret = FileWatcher::GetInstance().AddWatcherInfo(infoArg->fileName, infoArg); if (!ret) { + HILOGE("Failed to get add watcher info."); NError(EINVAL).ThrowErr(env); return nullptr; } -- Gitee From 79182940458e276f024719cb54a40099d63b3351 Mon Sep 17 00:00:00 2001 From: CaoChuan Date: Fri, 7 Jul 2023 08:36:15 +0800 Subject: [PATCH 12/12] code review Signed-off-by: CaoChuan --- .../mod_fs/class_watcher/watcher_entity.cpp | 141 +++++------------- .../src/mod_fs/class_watcher/watcher_entity.h | 3 +- .../kits/js/src/mod_fs/properties/watcher.cpp | 6 +- 3 files changed, 40 insertions(+), 110 deletions(-) diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp index cbfe68e03..221ed978c 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.cpp @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -53,7 +52,7 @@ bool FileWatcher::InitNotify() { notifyFd_ = inotify_init(); if (notifyFd_ < 0) { - HILOGE("Failed to init notify fail errCode:%{public}d", errno); + HILOGE("Failed to init notify errCode:%{public}d", errno); return false; } return true; @@ -83,59 +82,18 @@ vector FindUnWatchedEvent(const vector &allEvents, const uin return unWatchedEvents; } -bool FileWatcher::CheckEventWatched(const string &fileName, const uint32_t &event) -{ - auto iter = fileNameEventMap_.find(fileName); - if (iter == fileNameEventMap_.end()) { - return false; - } - vector allEvents = iter->second; - vector unWatchedEvents = FindUnWatchedEvent(allEvents, event); - if (unWatchedEvents.size() == 0) { - return true; - } - - return false; -} - -bool FileWatcher::CheckEventValid(const uint32_t &event) -{ - vector events = GetEvents(event); - return events.size() > 0 ? true : false; -} - bool FileWatcher::StartNotify(shared_ptr &arg) { lock_guard lock(watchMutex_); if (notifyFd_ < 0) { - HILOGE("Failed to start notify fail notifyFd_:%{public}d", notifyFd_); + HILOGE("Failed to start notify notifyFd_:%{public}d", notifyFd_); return false; } - if (!CheckEventValid(arg->events)) { - HILOGE("Failed to start notify, the watch event:%{public}x is wrong", arg->events); - errno = E_INVAL; - return false; - } - - if (CheckEventWatched(arg->fileName, arg->events)) { - for (auto iter = wdFileNameMap_.begin(); iter != wdFileNameMap_.end(); iter++) { - if (iter->second == arg->fileName) { - arg->wd = iter->first; - return true; - } - } - HILOGE("Failed to start notify, not find fileName:%{public}s", arg->fileName.c_str()); - return false; - } - vector watchEvents = ParseEvent(arg->fileName.c_str(), arg->events); - uint32_t watchEvent = 0; - for (const auto &event : watchEvents) { - watchEvent |= event; - } + uint32_t watchEvents = ParseEvent(arg->fileName.c_str(), arg->events); int wd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); if (wd < 0) { - HILOGE("Failed to start notify fail errCode:%{public}d", errno); + HILOGE("Failed to start notify errCode:%{public}d", errno); return false; } arg->wd = wd; @@ -160,42 +118,22 @@ bool FileWatcher::CheckFileWatched(const std::string &fileName, const uint32_t & void FileWatcher::RemoveEventFromMap(const shared_ptr &arg) { - auto watcherInfosIter = watcherInfoMap_.find(arg->fileName); - if (watcherInfosIter == watcherInfoMap_.end()) { - HILOGE("Failed to remove filename:%{public}s from map, it is not exists in the map", arg->fileName.c_str()); + auto eventIter = fileNameEventMap_.find(arg->fileName); + if (eventIter == fileNameEventMap_.end()) { + HILOGE("Failed to find filename:%{public}s from event map, because it does not exists in the map", + arg->fileName.c_str()); return; } - auto args = watcherInfosIter->second; - vector watchedEvent; - for (const auto &arg : args) { - uint32_t event = arg->events; - vector events = GetEvents(event); - watchedEvent.insert(watchedEvent.end(), events.begin(), events.end()); - } - sort(watchedEvent.begin(), watchedEvent.end()); - auto iter = unique(watchedEvent.begin(), watchedEvent.end()); - watchedEvent.erase(iter, watchedEvent.end()); - map>::iterator fileIter; - uint32_t removeEvent = arg->events; - vector removeEvents = GetEvents(removeEvent); - for (const auto &toRemoveEvent : removeEvents) { - auto watchedIter = find(watchedEvent.begin(), watchedEvent.end(), toRemoveEvent); - if (watchedIter != watchedEvent.end()) { - continue; - } - fileIter = fileNameEventMap_.find(arg->fileName); - if (fileIter == fileNameEventMap_.end()) { - continue; - } - auto eventIter = find(fileIter->second.begin(), fileIter->second.end(), toRemoveEvent); - if (eventIter == fileIter->second.end()) { - continue; + + for (int i = 0; i < eventIter->second.size(); i++) { + if (arg->events == eventIter->second[i]) { + eventIter->second.erase(eventIter->second.begin() + i); + break; } - fileIter->second.erase(eventIter); } - if (fileIter->second.size() == 0) { - fileNameEventMap_.erase(fileIter); + if (eventIter->second.size() == 0) { + fileNameEventMap_.erase(eventIter); } } @@ -209,14 +147,11 @@ bool FileWatcher::NotifyToWatchNewEvents(const shared_ptr &arg) if (eventIter == fileNameEventMap_.end()) { return true; } - vector watchEvents = eventIter->second; - uint32_t watchEvent = 0; - for (const auto &event : watchEvents) { - watchEvent |= event; - } + + uint32_t watchEvent = EventCombination(eventIter->second); int newWd = inotify_add_watch(notifyFd_, arg->fileName.c_str(), watchEvent); if (newWd < 0) { - HILOGE("Failed to start new notify fail errCode:%{public}d", errno); + HILOGE("Failed to start new notify errCode:%{public}d", errno); return false; } else { if (newWd != arg->wd) { @@ -241,14 +176,14 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) { unique_lock lock(watchMutex_); if (notifyFd_ < 0) { - HILOGE("Failed to stop notify fail notifyFd_:%{public}d", notifyFd_); + HILOGE("Failed to stop notify notifyFd_:%{public}d", notifyFd_); return false; } RemoveWatcherInfo(arg); if (FindWatcherByFileName(arg)) { bool ret = NotifyToWatchNewEvents(arg); if (!ret) { - HILOGE("Failed to stop notify combination new events error"); + HILOGE("Failed to combination new events"); return false; } } else { @@ -266,7 +201,7 @@ bool FileWatcher::StopNotify(const shared_ptr &arg) } string filename = iter->second; if (access(filename.c_str(), F_OK) == 0) { - HILOGE("Failed to stop notify fail errCode:%{public}d", errno); + HILOGE("Failed to stop notify errCode:%{public}d", errno); RemoveWatcherWdFileNameMap(arg->wd); return false; } @@ -337,27 +272,23 @@ bool FileWatcher::AddWatcherInfo(const string &fileName, const shared_ptr FileWatcher::ParseEvent(const string &fileName, const uint32_t &watcherEvent) +uint32_t EventCombination(vecotr events) +{ uint32_t events = 0; + for (const auto &event : events) { + events |= event + } + return events; +} + +uint32_t FileWatcher::ParseEvent(const string &fileName, const uint32_t &watcherEvent) { auto iter = fileNameEventMap_.find(fileName); if (iter == fileNameEventMap_.end()) { - vector events = GetEvents(watcherEvent); - fileNameEventMap_[fileName] = events; - return events; + fileNameEventMap_[fileName] = watcherEvent; + return watcherEvent; } else { - vector notWatchedEvents = FindUnWatchedEvent(iter->second, watcherEvent); - if (notWatchedEvents.size() > 0) { - vector newEvents; - newEvents.insert(newEvents.end(), iter->second.begin(), iter->second.end()); - newEvents.insert(newEvents.end(), notWatchedEvents.begin(), notWatchedEvents.end()); - sort(newEvents.begin(), newEvents.end()); - auto newEventIter = unique(newEvents.begin(), newEvents.end()); - newEvents.erase(newEventIter, newEvents.end()); - fileNameEventMap_[fileName] = newEvents; - return newEvents; - } else { - return iter->second; - } + iter->second.insert(watcherEvent); + return EventCombination(iter->second); } } @@ -365,13 +296,13 @@ void FileWatcher::RemoveWatcherInfo(const shared_ptr &arg) { auto fileNameIter = wdFileNameMap_.find(arg->wd); if (fileNameIter == wdFileNameMap_.end()) { - HILOGE("Failed to remove wd:%{public}d from map, it is not exist in the map", arg->wd); + HILOGE("Failed to remove wd:%{public}d from map, because it does not exist in the map", arg->wd); return; } string fileName = fileNameIter->second; auto iter = watcherInfoMap_.find(fileName); if (iter == watcherInfoMap_.end()) { - HILOGE("Failed to remove fileName:%{public}s from map, it is not exist in the map", fileName.c_str()); + HILOGE("Failed to remove fileName:%{public}s from map, because it does not exist in the map", fileName.c_str()); return; } auto infoArgs = iter->second; diff --git a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h index cad9926b5..c9fd6803c 100644 --- a/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h +++ b/interfaces/kits/js/src/mod_fs/class_watcher/watcher_entity.h @@ -68,8 +68,7 @@ private: void RemoveWatcherWdFileNameMap(const int &wd); void RemoveWatcherInfo(const std::shared_ptr &arg); int32_t GetWatcherInfoSize(const int &wd); - bool CheckEventWatched(const std::string &fileName, const uint32_t &event); - std::vector ParseEvent(const std::string &fileName, const uint32_t &watcherEvent); + uint32_t ParseEvent(const std::string &fileName, const uint32_t &watcherEvent); void NotifyEvent(const struct inotify_event *event, WatcherCallback callback); bool NotifyToWatchNewEvents(const std::shared_ptr &arg); bool CheckFileWatched(const std::string &fileName, const uint32_t &event); diff --git a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp index d351eec07..326ef43bf 100644 --- a/interfaces/kits/js/src/mod_fs/properties/watcher.cpp +++ b/interfaces/kits/js/src/mod_fs/properties/watcher.cpp @@ -41,7 +41,7 @@ namespace { static tuple CreateAndCheckForWatcherEntity(napi_env env, int &fd) { if (FileWatcher::GetInstance().GetNotifyId() < 0 && !FileWatcher::GetInstance().InitNotify()) { - HILOGE("Failed to get notifyId Or initnotify fail"); + HILOGE("Failed to get notifyId or initnotify fail"); return {nullptr, errno}; } napi_value objWatcher = NClass::InstantiateClass(env, WatcherNExporter::className_, {}); @@ -101,9 +101,9 @@ napi_value Watcher::CreateWatcher(napi_env env, napi_callback_info info) return nullptr; } - int errCode = -1; + int errCode = 0; auto infoArg = ParseParam(env, info, errCode); - if (infoArg == nullptr) { + if (errCode != 0) { HILOGE("Failed to parse param"); NError(errCode).ThrowErr(env); return nullptr; -- Gitee