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 <[email protected]>
Link: https://github.com/openwrt/odhcpd/pull/312
Signed-off-by: Álvaro Fernández Rojas <[email protected]>

diff --git a/src/config.c b/src/config.c
index 9246d64f23769beb91007537543147a04ba19287..a84021e46d7fbfc0f29ca132abeb68486a13a250 100644 (file)
--- 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 1f2417b909f95060faf02a02bcbe6b670f3543a2..bc0a833641443d549760fa7302e6ea0539e70fad 100644 (file)
--- 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 eadb0fc53c342cc73a3cd7821e659c7d38d9897e..f41aaa16d45e3e4d990bc1d22d0b54859ee7cac5 100644 (file)
--- 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);