Merge pull request #30 from spelcaster/fix-watcher-flow

fix the NotifierBuilder::run flow
erikzenker · Apr 5, 2018 · 1505377 · 1505377
2 parents a6fab42 + 0a9f043
commit 1505377
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 28 deletions.
diff --git 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<void(FileSystemEvent)> onEventTimeout);
   boost::optional<FileSystemEvent> getNextEvent();
   void stop();
+  bool hasStopped();
 
 private:
   fs::path wdToPath(int wd);

diff --git 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();
-}
+}
diff --git a/source/Inotify.cpp b/source/Inotify.cpp
@@ -189,7 +189,7 @@ boost::optional<FileSystemEvent> 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));
 
@@ -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) {

diff --git a/source/NotifierBuilder.cpp b/source/NotifierBuilder.cpp
@@ -4,19 +4,13 @@ namespace inotify {
 
 NotifierBuilder::NotifierBuilder()
     : mInotify(std::make_shared<Inotify>())
-    , 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<Event>(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();
     }
 }
 

diff --git 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(
-        [&notifier]() { BOOST_CHECK_THROW(notifier.runOnce(), std::runtime_error); });
-
-    openFile(testFile_);
-    thread.join();
-}
-
 BOOST_FIXTURE_TEST_CASE(shouldCallUserDefinedUnexpectedExceptionObserver, NotifierBuilderTests)
 {
     std::promise<void> 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();
-}
+}