odhcpd: simplify signal handling

Currently, odhcpd's main() function will:
 - call uloop_init()
 - set up some signal handlers for SIGUSR1/SIGINT/SIGTERM
   (SIGUSR1 is ignored, the other handlers call uloop_end())
 - call odhcpd_run(), which will:
 - set up some signal handlers for SIGTERM/SIGINT/SIGHUP
   (the first two call uloop_end(), SIGHUP calls odhcpd_reload())

Note that uloop_init() will call uloop_setup_signals() which will call
uloop_setup_signals() which will install handlers for SIGINT/SIGTERM
which will set uloop_cancelled to true (e.g. the same thing that
uloop_end() does).

In other words, the signal handlers are modified three times, and since
the default "exit" action is to call uloop_end(), there's nothing that
will actually react to signals until uloop_run() is called, meaning that
the startup will be uninterruptible if e.g. ubus_init() fails since
it'll just retry in a loop with no handling of signals.

So, simplify and fix this by letting uloop handle the signals and
checking if uloop has been told to exit, removing a bunch of
special-purpose code.

Signed-off-by: David Härdeman <david@hardeman.nu>
Link: https://github.com/openwrt/odhcpd/pull/312
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

3 files changed, 21 insertions, 44 deletions

diff --git a/src/config.c b/src/config.c
index 9246d64..a84021e 100644
--- a/src/config.c
+++ b/src/config.c
@@ -24,7 +24,6 @@
 #include "dhcpv6-pxe.h"
 
 static struct blob_buf b;
-static int reload_pipe[2] = { -1, -1 };
 
 static int lease_cfg_cmp(const void *k1, const void *k2, void *ptr);
 static void lease_cfg_update(struct vlist_tree *tree, struct vlist_node *node_new,
@@ -2433,40 +2432,28 @@ void odhcpd_reload(void)
 	uci_free_context(uci);
 }
 
-static void handle_signal(int signal)
+static void signal_reload(_o_unused struct uloop_signal *signal)
 {
-	char b[1] = {0};
-
-	if (signal == SIGHUP) {
-		if (write(reload_pipe[1], b, sizeof(b)) < 0) {}
-	} else
-		uloop_end();
-}
-
-static void reload_cb(struct uloop_fd *u, _o_unused unsigned int events)
-{
-	char b[512];
-	if (read(u->fd, b, sizeof(b)) < 0) {}
-
 	odhcpd_reload();
 }
 
-static struct uloop_fd reload_fd = { .fd = -1, .cb = reload_cb };
-
-void odhcpd_run(void)
+int odhcpd_run(void)
 {
-	if (pipe2(reload_pipe, O_NONBLOCK | O_CLOEXEC) < 0) {}
+	static struct uloop_signal sighup = { .signo = SIGHUP, .cb = signal_reload };
 
-	reload_fd.fd = reload_pipe[0];
-	uloop_fd_add(&reload_fd, ULOOP_READ);
-
-	signal(SIGTERM, handle_signal);
-	signal(SIGINT, handle_signal);
-	signal(SIGHUP, handle_signal);
-
-	while (ubus_init())
+	while (ubus_init()) {
+		if (uloop_cancelled)
+			return EXIT_FAILURE;
 		sleep(1);
+	}
 
 	odhcpd_reload();
+
+	/* uloop_init() already handles SIGINT/SIGTERM */
+	if (uloop_signal_add(&sighup) < 0)
+		return EXIT_FAILURE;
+
 	uloop_run();
+
+	return EXIT_SUCCESS;
 }
diff --git a/src/odhcpd.c b/src/odhcpd.c
index 1f2417b..bc0a833 100644
--- a/src/odhcpd.c
+++ b/src/odhcpd.c
@@ -67,11 +67,6 @@ void __iflog(int lvl, const char *fmt, ...)
 	va_end(ap);
 }
 
-static void sighandler(_o_unused int signal)
-{
-	uloop_end();
-}
-
 static void print_usage(const char *app)
 {
 	printf("== %s Usage ==\n"
@@ -146,6 +141,11 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (getuid() != 0) {
+		error("Must be run as root!");
+		return 2;
+	}
+
 	if (config.log_syslog) {
 		openlog("odhcpd", LOG_PERROR | LOG_PID, LOG_DAEMON);
 		setlogmask(LOG_UPTO(config.log_level));
@@ -153,19 +153,10 @@ int main(int argc, char **argv)
 
 	uloop_init();
 
-	if (getuid() != 0) {
-		error("Must be run as root!");
-		return 2;
-	}
-
 	ioctl_sock = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
 	if (ioctl_sock < 0)
 		return 4;
 
-	signal(SIGUSR1, SIG_IGN);
-	signal(SIGINT, sighandler);
-	signal(SIGTERM, sighandler);
-
 	if (netlink_init())
 		return 4;
 
@@ -187,8 +178,7 @@ int main(int argc, char **argv)
 		return 4;
 #endif
 
-	odhcpd_run();
-	return 0;
+	return odhcpd_run();
 }
 
 
diff --git a/src/odhcpd.h b/src/odhcpd.h
index eadb0fc..f41aaa1 100644
--- a/src/odhcpd.h
+++ b/src/odhcpd.h
@@ -536,7 +536,7 @@ int odhcpd_get_flags(const struct interface *iface);
 struct interface* odhcpd_get_interface_by_index(int ifindex);
 void odhcpd_urandom(void *data, size_t len);
 
-void odhcpd_run(void);
+int odhcpd_run(void);
 time_t odhcpd_time(void);
 ssize_t odhcpd_unhexlify(uint8_t *dst, size_t len, const char *src);
 void odhcpd_hexlify(char *dst, const uint8_t *src, size_t len);