systemd_watchdog: systemd watchdog object and thread_factory cleanups

Static code analysis fix (preparation) _004 (Issue Avnu#40)

Make SystemdWatchdogHandler object to be created in main() for proper cleanup during exit.
watchodg_setup() changed to a public member of SystemdWatchdogHandler class.

Author: PawelModrzejewski

linux/src/daemon_cl.cpp

@@ -58,7 +58,7 @@
 #include <string.h>
 
 #ifdef SYSTEMD_WATCHDOG
-#include <watchdog.hpp>
+#include "watchdog.hpp"
 #endif
 
 #define PHY_DELAY_GB_TX_I20 184 //1G delay
@@ -100,34 +100,6 @@ void print_usage( char *arg0 ) {
 		);
 }
 
-int watchdog_setup(OSThreadFactory *thread_factory)
-{
-#ifdef SYSTEMD_WATCHDOG
-	SystemdWatchdogHandler *watchdog = new SystemdWatchdogHandler();
-	OSThread *watchdog_thread = thread_factory->createThread();
-	int watchdog_result;
-	long unsigned int watchdog_interval;
-	watchdog_interval = watchdog->getSystemdWatchdogInterval(&watchdog_result);
-	if (watchdog_result) {
-		GPTP_LOG_INFO("Watchtog interval read from service file: %lu us", watchdog_interval);
-		watchdog->update_interval = watchdog_interval / 2;
-		GPTP_LOG_STATUS("Starting watchdog handler (Update every: %lu us)", watchdog->update_interval);
-		watchdog_thread->start(watchdogUpdateThreadFunction, watchdog);
-		return 0;
-	} else if (watchdog_result < 0) {
-		GPTP_LOG_ERROR("Watchdog settings read error.");
-		delete watchdog;
-		return -1;
-	} else {
-		GPTP_LOG_STATUS("Watchdog disabled");
-		delete watchdog;
-		return 0;
-	}
-#else
-	return 0;
-#endif
-}
-
 static IEEE1588Clock *pClock = NULL;
 static EtherPort *pPort = NULL;
 
@@ -156,7 +128,6 @@ int main(int argc, char **argv)
 	memset(config_file_path, 0, 512);
 
 	GPTPPersist *pGPTPPersist = NULL;
-	LinuxThreadFactory *thread_factory = new LinuxThreadFactory();
 
 	// Block SIGUSR1
 	{
@@ -171,10 +142,22 @@ int main(int argc, char **argv)
 
 	GPTP_LOG_REGISTER();
 	GPTP_LOG_INFO("gPTP starting");
-	if (watchdog_setup(thread_factory) != 0) {
+
+	LinuxThreadFactory *thread_factory = new LinuxThreadFactory();
+
+#ifdef SYSTEMD_WATCHDOG
+	SystemdWatchdogHandler *watchdog = new SystemdWatchdogHandler();
+	if (watchdog->watchdog_setup(thread_factory) != 0) {
 		GPTP_LOG_ERROR("Watchdog handler setup error");
+		delete watchdog;
+		delete thread_factory;
 		return -1;
 	}
+#else
+	// dummy pointer for CLEANUP macro
+	int *watchdog = NULL;
+#endif
+
 	phy_delay_map_t ether_phy_delay;
 	bool input_delay=false;
 

linux/src/watchdog.cpp

@@ -1,5 +1,4 @@
 #include "watchdog.hpp"
-#include "avbts_osthread.hpp"
 #include "gptp_log.hpp"
 #include <systemd/sd-daemon.h>
 
@@ -11,17 +10,23 @@ OSThreadExitCode watchdogUpdateThreadFunction(void *arg)
 	return osthread_ok;
 }
 
+bool SystemdWatchdogHandler::startSystemdWatchdogThread(OSThreadFactory *thread_factory)
+{
+	watchdog_thread = thread_factory->createThread();
+	return watchdog_thread->start(watchdogUpdateThreadFunction, this);
+}
 
 SystemdWatchdogHandler::SystemdWatchdogHandler()
 {
 	GPTP_LOG_INFO("Creating Systemd watchdog handler.");
 	LinuxTimerFactory timer_factory = LinuxTimerFactory();
-	timer = timer_factory.createTimer();
+	watchdog_timer = timer_factory.createTimer();
 }
 
 SystemdWatchdogHandler::~SystemdWatchdogHandler()
 {
-	//Do nothing
+	delete watchdog_timer;
+	delete watchdog_thread;
 }
 
 long unsigned int
@@ -39,8 +44,37 @@ void SystemdWatchdogHandler::run_update()
 		GPTP_LOG_DEBUG("NOTIFYING WATCHDOG.");
 		sd_notify(0, "WATCHDOG=1");
 		GPTP_LOG_DEBUG("GOING TO SLEEP %lld", update_interval);
-		timer->sleep(update_interval);
+		watchdog_timer->sleep(update_interval);
 		GPTP_LOG_DEBUG("WATCHDOG WAKE UP");
 	}
 }
 
+int SystemdWatchdogHandler::watchdog_setup(OSThreadFactory *thread_factory)
+{
+	int watchdog_result;
+	long unsigned int watchdog_interval;
+
+	if (!thread_factory) {
+		GPTP_LOG_ERROR("Watchog setup invalid argument (thread_factory)");
+		return -1;
+	}
+
+	watchdog_interval = getSystemdWatchdogInterval(&watchdog_result);
+	if (watchdog_result) {
+		GPTP_LOG_INFO("Watchdog interval read from service file: %lu us", watchdog_interval);
+		update_interval = watchdog_interval / 2;
+		GPTP_LOG_STATUS("Starting watchdog handler (Update every: %lu us)", update_interval);
+		if (startSystemdWatchdogThread(thread_factory)) {
+			return 0;
+		} else {
+			GPTP_LOG_ERROR("Starting watchdog thread failed");
+			return -1;
+		}
+	} else if (watchdog_result < 0) {
+		GPTP_LOG_ERROR("Watchdog settings read error.");
+		return -1;
+	} else {
+		GPTP_LOG_STATUS("Watchdog disabled");
+		return 0;
+	}
+}

linux/src/watchdog.hpp

@@ -2,20 +2,23 @@
 #define SYSTEMDWATCHDOGHANDLER_H
 #include <linux_hal_common.hpp>
 #include <avbts_ostimer.hpp>
-
+#include <avbts_osthread.hpp>
 
 OSThreadExitCode watchdogUpdateThreadFunction(void *arg);
 
 class SystemdWatchdogHandler
 {
 public:
-	long unsigned int update_interval;
-	long unsigned int getSystemdWatchdogInterval(int *result);
+	int watchdog_setup(OSThreadFactory *thread_factory);
 	void run_update();
 	SystemdWatchdogHandler();
 	virtual ~SystemdWatchdogHandler();
 private:
-	OSTimer *timer;
+	OSTimer *watchdog_timer;
+	OSThread *watchdog_thread;
+	long unsigned int update_interval;
+	long unsigned int getSystemdWatchdogInterval(int *result);
+	bool startSystemdWatchdogThread(OSThreadFactory *thread_factory);
 };
 
 #endif // SYSTEMDWATCHDOGHANDLER_H