From 0ccc4ad2524a289752bc682fac44309dce951188 Mon Sep 17 00:00:00 2001 From: Hugo Augusto Freitas do Carmo Date: Mon, 26 Mar 2018 16:59:38 -0300 Subject: [PATCH 1/4] fix memset in Inotify::getNextEvent - Although the previous version was working, it was incorrect and it sometimes raised the SIGABRT signal; Co-authored-by: Igor de Souza Santos --- source/Inotify.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Inotify.cpp b/source/Inotify.cpp index daaf271..6237ac2 100644 --- a/source/Inotify.cpp +++ b/source/Inotify.cpp @@ -189,7 +189,7 @@ boost::optional Inotify::getNextEvent() // Read Events from fd into buffer while (mEventQueue.empty()) { length = 0; - memset(&buffer, 0, EVENT_BUF_LEN); + memset(buffer, '\0', sizeof(buffer)); while (length <= 0 && !stopped) { std::this_thread::sleep_for(std::chrono::milliseconds(mThreadSleep)); From 3d175b5cd2b275071571e040af7d52ddcac6ba1f Mon Sep 17 00:00:00 2001 From: Hugo Augusto Freitas do Carmo Date: Tue, 27 Mar 2018 10:59:13 -0300 Subject: [PATCH 2/4] add the method Inotify::hasStopped - This method will allow the NotifierBuilder to test if the watcher was flagged to stop it's execution, hence stopping its execution; --- include/inotify-cpp/Inotify.h | 1 + source/Inotify.cpp | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/include/inotify-cpp/Inotify.h b/include/inotify-cpp/Inotify.h index 6b4398e..f5b3f86 100755 --- a/include/inotify-cpp/Inotify.h +++ b/include/inotify-cpp/Inotify.h @@ -84,6 +84,7 @@ class Inotify { void setEventTimeout(std::chrono::milliseconds eventTimeout, std::function onEventTimeout); boost::optional getNextEvent(); void stop(); + bool hasStopped(); private: fs::path wdToPath(int wd); diff --git a/source/Inotify.cpp b/source/Inotify.cpp index 6237ac2..bd37922 100644 --- a/source/Inotify.cpp +++ b/source/Inotify.cpp @@ -262,6 +262,11 @@ void Inotify::stop() stopped = true; } +bool Inotify::hasStopped() +{ + return stopped; +} + bool Inotify::isIgnored(std::string file) { for (unsigned i = 0; i < mOnceIgnoredDirectories.size(); ++i) { From 1fa693795ba4a522f3e5fab86f9a98a980073943 Mon Sep 17 00:00:00 2001 From: Hugo Augusto Freitas do Carmo Date: Tue, 27 Mar 2018 11:02:40 -0300 Subject: [PATCH 3/4] fix the NotifierBuilder::run flow - The attribute mUnexpectedEventObserver will not be configured in the NotifierBuilder constructor; - the return value for runOnce is now void; - The method run now checks if the mInotify was flagged to stop its execution, instead of depending on the return value of the runOnce; - The mUnexpectedEventObserver will be called only if the user has it configured, and calling mUnexpectedEventObserver will not stop the watcher; --- include/inotify-cpp/NotifierBuilder.h | 4 ++-- source/NotifierBuilder.cpp | 25 ++++++++++++------------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/inotify-cpp/NotifierBuilder.h b/include/inotify-cpp/NotifierBuilder.h index 4294945..108532c 100644 --- a/include/inotify-cpp/NotifierBuilder.h +++ b/include/inotify-cpp/NotifierBuilder.h @@ -17,7 +17,7 @@ class NotifierBuilder { NotifierBuilder(); auto run() -> void; - auto runOnce() -> bool; + auto runOnce() -> void; auto stop() -> void; auto watchPathRecursively(boost::filesystem::path path) -> NotifierBuilder&; auto watchFile(boost::filesystem::path file) -> NotifierBuilder&; @@ -37,4 +37,4 @@ class NotifierBuilder { }; NotifierBuilder BuildNotifier(); -} \ No newline at end of file +} diff --git a/source/NotifierBuilder.cpp b/source/NotifierBuilder.cpp index f059aa9..651ead7 100644 --- a/source/NotifierBuilder.cpp +++ b/source/NotifierBuilder.cpp @@ -4,19 +4,13 @@ namespace inotify { NotifierBuilder::NotifierBuilder() : mInotify(std::make_shared()) - , mUnexpectedEventObserver([](Notification notification) { - std::stringstream ss; - ss << "Unexpected event " << notification.event << " on " << notification.path; - throw std::runtime_error(ss.str()); - }) - { } NotifierBuilder BuildNotifier() { return {}; -}; +} auto NotifierBuilder::watchPathRecursively(boost::filesystem::path path) -> NotifierBuilder& { @@ -87,11 +81,11 @@ auto NotifierBuilder::setEventTimeout( return *this; } -auto NotifierBuilder::runOnce() -> bool +auto NotifierBuilder::runOnce() -> void { auto fileSystemEvent = mInotify->getNextEvent(); if (!fileSystemEvent) { - return false; + return; } Event event = static_cast(fileSystemEvent->mask); @@ -102,8 +96,11 @@ auto NotifierBuilder::runOnce() -> bool auto eventAndEventObserver = mEventObserver.find(event); if (eventAndEventObserver == mEventObserver.end()) { - mUnexpectedEventObserver(notification); - return true; + if (mUnexpectedEventObserver) { + mUnexpectedEventObserver(notification); + } + + return; } auto eventObserver = eventAndEventObserver->second; @@ -113,9 +110,11 @@ auto NotifierBuilder::runOnce() -> bool auto NotifierBuilder::run() -> void { while (true) { - if (!runOnce()) { - return; + if (mInotify->hasStopped()) { + break; } + + runOnce(); } } From 0a9f04386f93bfa41d28ed5d5635d03fb0d3bff3 Mon Sep 17 00:00:00 2001 From: Hugo Augusto Freitas do Carmo Date: Tue, 27 Mar 2018 11:13:08 -0300 Subject: [PATCH 4/4] remove the shouldThrowOnUnexpectedEvent test case - This is not the default behavior anymore; --- test/unit/NotifierBuilderTests.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/test/unit/NotifierBuilderTests.cpp b/test/unit/NotifierBuilderTests.cpp index 7fd58e7..4f38e3c 100644 --- a/test/unit/NotifierBuilderTests.cpp +++ b/test/unit/NotifierBuilderTests.cpp @@ -188,17 +188,6 @@ BOOST_FIXTURE_TEST_CASE(shouldUnwatchPath, NotifierBuilderTests) thread.join(); } -BOOST_FIXTURE_TEST_CASE(shouldThrowOnUnexpectedEvent, NotifierBuilderTests) -{ - auto notifier = BuildNotifier().watchFile(testFile_); - - std::thread thread( - [¬ifier]() { BOOST_CHECK_THROW(notifier.runOnce(), std::runtime_error); }); - - openFile(testFile_); - thread.join(); -} - BOOST_FIXTURE_TEST_CASE(shouldCallUserDefinedUnexpectedExceptionObserver, NotifierBuilderTests) { std::promise observerCalled; @@ -238,4 +227,4 @@ BOOST_FIXTURE_TEST_CASE(shouldSetEventTimeout, NotifierBuilderTests) BOOST_CHECK(promisedOpen_.get_future().wait_for(timeout_) == std::future_status::ready); BOOST_CHECK(timeoutObserved.get_future().wait_for(timeout_) == std::future_status::ready); thread.join(); -} \ No newline at end of file +}