[PATCH v7 02/12] daemon: libify child process handling functions

["Matthew John Cheetham via GitGitGadget" <gitgitgadget@gmail.com>]

From: Matthew John Cheetham <mjcheetham@outlook.com>

Extract functions and structures for managing child processes started
from the parent daemon-like process from `daemon.c` to the new shared
`daemon-utils.{c,h}` files.

One minor functional change is introduced to `check_dead_children()`
where the logging of a dead/disconnected child is now optional. With the
'libification' of these functions we extract the call to `loginfo` to a
call to a function pointer, and guard the log message creation and
logging behind a `NULL` check. Callers can now skip logging by passing
`NULL` as the `log_fn loginfo` argument.
The behaviour of callers in `daemon.c` remains the same (save one extra
NULL check)  however as a pointer to `loginfo` is always passed.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
 daemon-utils.c | 77 ++++++++++++++++++++++++++++++++++++++++++
 daemon-utils.h | 32 ++++++++++++++++++
 daemon.c       | 92 +++-----------------------------------------------
 3 files changed, 114 insertions(+), 87 deletions(-)

diff --git a/daemon-utils.c b/daemon-utils.c
index b96b55962db..8506664b440 100644
--- a/daemon-utils.c
+++ b/daemon-utils.c
@@ -207,3 +207,80 @@ void socksetup(struct string_list *listen_addr, int listen_port,
 		}
 	}
 }
+
+static int addrcmp(const struct sockaddr_storage *s1,
+    const struct sockaddr_storage *s2)
+{
+	const struct sockaddr *sa1 = (const struct sockaddr*) s1;
+	const struct sockaddr *sa2 = (const struct sockaddr*) s2;
+
+	if (sa1->sa_family != sa2->sa_family)
+		return sa1->sa_family - sa2->sa_family;
+	if (sa1->sa_family == AF_INET)
+		return memcmp(&((struct sockaddr_in *)s1)->sin_addr,
+		    &((struct sockaddr_in *)s2)->sin_addr,
+		    sizeof(struct in_addr));
+#ifndef NO_IPV6
+	if (sa1->sa_family == AF_INET6)
+		return memcmp(&((struct sockaddr_in6 *)s1)->sin6_addr,
+		    &((struct sockaddr_in6 *)s2)->sin6_addr,
+		    sizeof(struct in6_addr));
+#endif
+	return 0;
+}
+
+void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen,
+	       struct child *firstborn , unsigned int *live_children)
+{
+	struct child *newborn, **cradle;
+
+	CALLOC_ARRAY(newborn, 1);
+	(*live_children)++;
+	memcpy(&newborn->cld, cld, sizeof(*cld));
+	memcpy(&newborn->address, addr, addrlen);
+	for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
+		if (!addrcmp(&(*cradle)->address, &newborn->address))
+			break;
+	newborn->next = *cradle;
+	*cradle = newborn;
+}
+
+void kill_some_child(struct child *firstborn)
+{
+	const struct child *blanket, *next;
+
+	if (!(blanket = firstborn))
+		return;
+
+	for (; (next = blanket->next); blanket = next)
+		if (!addrcmp(&blanket->address, &next->address)) {
+			kill(blanket->cld.pid, SIGTERM);
+			break;
+		}
+}
+
+void check_dead_children(struct child *firstborn, unsigned int *live_children,
+			 log_fn loginfo)
+{
+	int status;
+	pid_t pid;
+
+	struct child **cradle, *blanket;
+	for (cradle = &firstborn; (blanket = *cradle);)
+		if ((pid = waitpid(blanket->cld.pid, &status, WNOHANG)) > 1) {
+			if (loginfo) {
+				const char *dead = "";
+				if (status)
+					dead = " (with error)";
+				loginfo("[%"PRIuMAX"] Disconnected%s",
+					(uintmax_t)pid, dead);
+			}
+
+			/* remove the child */
+			*cradle = blanket->next;
+			(*live_children)--;
+			child_process_clear(&blanket->cld);
+			free(blanket);
+		} else
+			cradle = &blanket->next;
+}
diff --git a/daemon-utils.h b/daemon-utils.h
index 6710a2a6dc0..97e5cae20b8 100644
--- a/daemon-utils.h
+++ b/daemon-utils.h
@@ -2,6 +2,7 @@
 #define DAEMON_UTILS_H
 
 #include "git-compat-util.h"
+#include "run-command.h"
 #include "string-list.h"
 
 typedef void (*log_fn)(const char *msg, ...);
@@ -20,4 +21,35 @@ void socksetup(struct string_list *listen_addr, int listen_port,
 	       struct socketlist *socklist, int reuseaddr,
 	       log_fn logerror);
 
+struct child {
+	struct child *next;
+	struct child_process cld;
+	struct sockaddr_storage address;
+};
+
+/*
+ * Add the child_process to the set of children and increment the number of
+ * live children.
+ */
+void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen,
+	       struct child *firstborn, unsigned int *live_children);
+
+/*
+ * Kill the newest connection from a duplicate IP.
+ *
+ * This function should be called if the number of connections grows
+ * past the maximum number of allowed connections.
+ */
+void kill_some_child(struct child *firstborn);
+
+/*
+ * Check for children that have disconnected and remove them from the
+ * active set, decrementing the number of live children.
+ *
+ * Optionally log the child PID that disconnected by passing a loginfo
+ * function.
+ */
+void check_dead_children(struct child *firstborn, unsigned int *live_children,
+			 log_fn loginfo);
+
 #endif
diff --git a/daemon.c b/daemon.c
index 1ed4e705680..ec3b407ecbc 100644
--- a/daemon.c
+++ b/daemon.c
@@ -785,93 +785,11 @@ static int execute(void)
 	return -1;
 }
 
-static int addrcmp(const struct sockaddr_storage *s1,
-    const struct sockaddr_storage *s2)
-{
-	const struct sockaddr *sa1 = (const struct sockaddr*) s1;
-	const struct sockaddr *sa2 = (const struct sockaddr*) s2;
-
-	if (sa1->sa_family != sa2->sa_family)
-		return sa1->sa_family - sa2->sa_family;
-	if (sa1->sa_family == AF_INET)
-		return memcmp(&((struct sockaddr_in *)s1)->sin_addr,
-		    &((struct sockaddr_in *)s2)->sin_addr,
-		    sizeof(struct in_addr));
-#ifndef NO_IPV6
-	if (sa1->sa_family == AF_INET6)
-		return memcmp(&((struct sockaddr_in6 *)s1)->sin6_addr,
-		    &((struct sockaddr_in6 *)s2)->sin6_addr,
-		    sizeof(struct in6_addr));
-#endif
-	return 0;
-}
-
 static int max_connections = 32;
 
 static unsigned int live_children;
 
-static struct child {
-	struct child *next;
-	struct child_process cld;
-	struct sockaddr_storage address;
-} *firstborn;
-
-static void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen)
-{
-	struct child *newborn, **cradle;
-
-	CALLOC_ARRAY(newborn, 1);
-	live_children++;
-	memcpy(&newborn->cld, cld, sizeof(*cld));
-	memcpy(&newborn->address, addr, addrlen);
-	for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
-		if (!addrcmp(&(*cradle)->address, &newborn->address))
-			break;
-	newborn->next = *cradle;
-	*cradle = newborn;
-}
-
-/*
- * This gets called if the number of connections grows
- * past "max_connections".
- *
- * We kill the newest connection from a duplicate IP.
- */
-static void kill_some_child(void)
-{
-	const struct child *blanket, *next;
-
-	if (!(blanket = firstborn))
-		return;
-
-	for (; (next = blanket->next); blanket = next)
-		if (!addrcmp(&blanket->address, &next->address)) {
-			kill(blanket->cld.pid, SIGTERM);
-			break;
-		}
-}
-
-static void check_dead_children(void)
-{
-	int status;
-	pid_t pid;
-
-	struct child **cradle, *blanket;
-	for (cradle = &firstborn; (blanket = *cradle);)
-		if ((pid = waitpid(blanket->cld.pid, &status, WNOHANG)) > 1) {
-			const char *dead = "";
-			if (status)
-				dead = " (with error)";
-			loginfo("[%"PRIuMAX"] Disconnected%s", (uintmax_t)pid, dead);
-
-			/* remove the child */
-			*cradle = blanket->next;
-			live_children--;
-			child_process_clear(&blanket->cld);
-			free(blanket);
-		} else
-			cradle = &blanket->next;
-}
+static struct child *firstborn;
 
 static struct strvec cld_argv = STRVEC_INIT;
 static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
@@ -879,9 +797,9 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 	struct child_process cld = CHILD_PROCESS_INIT;
 
 	if (max_connections && live_children >= max_connections) {
-		kill_some_child();
+		kill_some_child(firstborn);
 		sleep(1);  /* give it some time to die */
-		check_dead_children();
+		check_dead_children(firstborn, &live_children, loginfo);
 		if (live_children >= max_connections) {
 			close(incoming);
 			logerror("Too many children, dropping connection");
@@ -914,7 +832,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 	if (start_command(&cld))
 		logerror("unable to fork");
 	else
-		add_child(&cld, addr, addrlen);
+		add_child(&cld, addr, addrlen, firstborn, &live_children);
 }
 
 static void child_handler(int signo)
@@ -944,7 +862,7 @@ static int service_loop(struct socketlist *socklist)
 	for (;;) {
 		int i;
 
-		check_dead_children();
+		check_dead_children(firstborn, &live_children, loginfo);
 
 		if (poll(pfd, socklist->nr, -1) < 0) {
 			if (errno != EINTR) {
-- 
gitgitgadget