watchdog: Rewrite timeoutHandler() to make disabling part of the timeout

This makes no functional changes to the user interface but make future code
reworks shorter.

Change-Id: Ibd57a5d1090588c8a7b2a67730660c3cf47c832e
Signed-off-by: William A. Kennington III <wak@google.com>

4 files changed, 58 insertions, 54 deletions

diff --git a/mainapp.cpp b/mainapp.cpp
index 845e291..d935bfb 100644
--- a/mainapp.cpp
+++ b/mainapp.cpp
@@ -159,8 +159,8 @@ int main(int argc, char** argv)
         // Claim the bus
         bus.request_name(service.c_str());
 
-        // Loop forever processing events
-        while (true)
+        // Loop until our timer expires and we don't want to continue
+        while (continueAfterTimeout || !watchdog.timerExpired())
         {
             // -1 denotes wait for ever
             r = sd_event_run(eventP.get(), (uint64_t)-1);
@@ -169,23 +169,6 @@ int main(int argc, char** argv)
                 log<level::ERR>("Error waiting for events");
                 elog<InternalFailure>();
             }
-
-            // The timer expiring is an event that breaks from the above.
-            if (watchdog.timerExpired())
-            {
-                // Either disable the timer or exit.
-                if (continueAfterTimeout)
-                {
-                    // The watchdog will be disabled but left running to be
-                    // re-enabled.
-                    watchdog.enabled(false);
-                }
-                else
-                {
-                    // The watchdog daemon will now exit.
-                    break;
-                }
-            }
         }
     }
     catch(InternalFailure& e)
diff --git a/test/watchdog_test.cpp b/test/watchdog_test.cpp
index 90ec161..a749435 100644
--- a/test/watchdog_test.cpp
+++ b/test/watchdog_test.cpp
@@ -165,7 +165,7 @@ TEST_F(WdogTest, enableWdogAndWaitTillEnd)
     // Waiting default expiration
     EXPECT_EQ(expireTime - 1s, waitForWatchdog(expireTime));
 
-    EXPECT_TRUE(wdog->enabled());
+    EXPECT_FALSE(wdog->enabled());
     EXPECT_EQ(0, wdog->timeRemaining());
     EXPECT_TRUE(wdog->timerExpired());
     EXPECT_FALSE(wdog->timerEnabled());
diff --git a/watchdog.cpp b/watchdog.cpp
index 12631a6..24e5416 100644
--- a/watchdog.cpp
+++ b/watchdog.cpp
@@ -17,29 +17,30 @@ constexpr auto SYSTEMD_INTERFACE  = "org.freedesktop.systemd1.Manager";
 // Enable or disable watchdog
 bool Watchdog::enabled(bool value)
 {
-    if (this->enabled() != value)
+    if (!value)
     {
-        if (value)
-        {
-            // Start ONESHOT timer. Timer handles all in usec
-            auto usec = duration_cast<microseconds>(
-                                      milliseconds(this->interval()));
-            // Update new expiration
-            timer.start(usec);
-
-            // Enable timer
-            timer.setEnabled<std::true_type>();
-
-            log<level::INFO>("watchdog: enabled and started",
-                             entry("INTERVAL=%llu", this->interval()));
-        }
-        else
-        {
-            timer.setEnabled<std::false_type>();
-            timer.clearExpired();
-            log<level::INFO>("watchdog: disabled");
-        }
+        // Attempt to disable our timer if needed
+        tryDisable();
+
+        return false;
     }
+    else if (!this->enabled())
+    {
+        // Start ONESHOT timer. Timer handles all in usec
+        auto usec = duration_cast<microseconds>(
+                milliseconds(this->interval()));
+
+        // Update new expiration
+        timer.clearExpired();
+        timer.start(usec);
+
+        // Enable timer
+        timer.setEnabled<std::true_type>();
+
+        log<level::INFO>("watchdog: enabled and started",
+                entry("INTERVAL=%llu", this->interval()));
+    }
+
     return WatchdogInherits::enabled(value);
 }
 
@@ -95,20 +96,37 @@ void Watchdog::timeOutHandler()
     {
         log<level::INFO>("watchdog: Timed out with no target",
                 entry("ACTION=%s", convertForMessage(action).c_str()));
-        return;
+    }
+    else
+    {
+        auto method = bus.new_method_call(SYSTEMD_SERVICE,
+                SYSTEMD_ROOT,
+                SYSTEMD_INTERFACE,
+                "StartUnit");
+        method.append(target->second);
+        method.append("replace");
+
+        log<level::INFO>("watchdog: Timed out",
+                entry("ACTION=%s", convertForMessage(action).c_str()),
+                entry("TARGET=%s", target->second.c_str()));
+        bus.call_noreply(method);
+    }
+
+    tryDisable();
+}
+
+void Watchdog::tryDisable()
+{
+    if (timerEnabled())
+    {
+        timer.setEnabled<std::false_type>();
+
+        log<level::INFO>("watchdog: disabled");
     }
 
-    auto method = bus.new_method_call(SYSTEMD_SERVICE,
-            SYSTEMD_ROOT,
-            SYSTEMD_INTERFACE,
-            "StartUnit");
-    method.append(target->second);
-    method.append("replace");
-
-    log<level::INFO>("watchdog: Timed out",
-            entry("ACTION=%s", convertForMessage(action).c_str()),
-            entry("TARGET=%s", target->second.c_str()));
-    bus.call_noreply(method);
+    // Make sure we accurately reflect our enabled state to the
+    // dbus interface.
+    WatchdogInherits::enabled(false);
 }
 
 } // namespace watchdog
diff --git a/watchdog.hpp b/watchdog.hpp
index e829699..ee5e65d 100644
--- a/watchdog.hpp
+++ b/watchdog.hpp
@@ -72,7 +72,7 @@ class Watchdog : public WatchdogInherits
 
         /** @brief Gets the remaining time before watchdog expires.
          *
-         *  @return 0 if watchdog is disabled or expired.
+         *  @return 0 if watchdog is expired.
          *          Remaining time in milliseconds otherwise.
          */
         uint64_t timeRemaining() const override;
@@ -111,6 +111,9 @@ class Watchdog : public WatchdogInherits
 
         /** @brief Optional Callback handler on timer expirartion */
         void timeOutHandler();
+
+        /** @brief Attempt to disable the watchdog if needed */
+        void tryDisable();
 };
 
 } // namespace watchdog