[PATCH 1/3] daemon.c: minor style fixup
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:13
Subsystem:
the rest · Maintainer:
Linus Torvalds
* "else" on the same line as "}" that closes corresponding "if (...) {";
* multi-line comments begin with "/*\n";
* sizeof, even it is not a function, is written as "sizeof(...)";
* no need to check x?alloc() return value -- it would have died;
* "if (...) { ... }" that covers the whole function body can be dedented
by returning from the function early with "if (!...) return;";
* SP on each side of an operator, i.e. "a > 0", not "a>0";
Also removes stale comment describing how remove_child() used to do its
thing.
Signed-off-by: Junio C Hamano <redacted>:
---
Junio C Hamano [off-list ref] writes:
I however do have one issue with the logic, though.
It seems that kill_some_child() will not kill anything if nobody else is
coming from the same address, while the old code did kill some. Is this
intended?
By the way, add_child() compares the whole "struct sockaddr_storage" in
order to queue the newborn in front of an existing connection from the
same address, and kill_some_child() takes advanrage of this to kill the
newest connection ("We kill the newest" comment should probably be moved
to add_child() to describe why the queuing is done this way). If you
simplify add_child() to queue the newborn always at the front of the list,
your kill_some_child() will continue to do so, so I do not see the point
of the loop in add_child(). Am I mistaken?So here is a three-patch series on top. I am not sure about the last one. daemon.c | 72 +++++++++++++++++++++++++------------------------------------ 1 files changed, 30 insertions(+), 42 deletions(-)
diff --git a/daemon.c b/daemon.c
index 79f0388..35bd34a 100644
--- a/daemon.c
+++ b/daemon.c@@ -81,9 +81,9 @@ static void logreport(int priority, const char *err, va_list params) char buf[1024]; vsnprintf(buf, sizeof(buf), err, params); syslog(priority, "%s", buf); - } - else { - /* Since stderr is set to linebuffered mode, the + } else { + /* + * Since stderr is set to linebuffered mode, the * logging of different processes will not overlap */ fprintf(stderr, "[%d] ", (int)getpid());
@@ -596,31 +596,20 @@ static struct child { static void add_child(pid_t pid, struct sockaddr *addr, int addrlen) { - struct child *newborn; - newborn = xcalloc(1, sizeof *newborn); - if (newborn) { - struct child **cradle; - - live_children++; - newborn->pid = pid; - memcpy(&newborn->address, addr, addrlen); - for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next) - if (!memcmp(&(*cradle)->address, &newborn->address, - sizeof newborn->address)) - break; - newborn->next = *cradle; - *cradle = newborn; - } - else - logerror("Out of memory spawning new child"); + struct child *newborn, **cradle; + newborn = xcalloc(1, sizeof(*newborn)); + + live_children++; + newborn->pid = pid; + memcpy(&newborn->address, addr, addrlen); + for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next) + if (!memcmp(&(*cradle)->address, &newborn->address, + sizeof(newborn->address))) + break; + newborn->next = *cradle; + *cradle = newborn; } -/* - * Walk from "deleted" to "spawned", and remove child "pid". - * - * We move everything up by one, since the new "deleted" will - * be one higher. - */ static void remove_child(pid_t pid) { struct child **cradle, *blanket;
@@ -642,18 +631,17 @@ static void remove_child(pid_t pid) */ static void kill_some_child(void) { - const struct child *blanket; + const struct child *blanket, *next; - if ((blanket = firstborn)) { - const struct child *next; + if (!(blanket = firstborn)) + return; - for (; (next = blanket->next); blanket = next) - if (!memcmp(&blanket->address, &next->address, - sizeof next->address)) { - kill(blanket->pid, SIGTERM); - break; - } - } + for (; (next = blanket->next); blanket = next) + if (!memcmp(&blanket->address, &next->address, + sizeof(next->address))) { + kill(blanket->pid, SIGTERM); + break; + } } static void check_dead_children(void)
@@ -661,10 +649,10 @@ static void check_dead_children(void) int status; pid_t pid; - while ((pid = waitpid(-1, &status, WNOHANG))>0) { + while ((pid = waitpid(-1, &status, WNOHANG)) > 0) { const char *dead = ""; remove_child(pid); - if (!WIFEXITED(status) || WEXITSTATUS(status) > 0) + if (!WIFEXITED(status) || (WEXITSTATUS(status) > 0)) dead = " (with error)"; loginfo("[%d] Disconnected%s", (int)pid, dead); }
@@ -676,7 +664,7 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen) if (max_connections && live_children >= max_connections) { kill_some_child(); - sleep(1); /* give it some time to die */ + sleep(1); /* give it some time to die */ check_dead_children(); if (live_children >= max_connections) { close(incoming);
@@ -705,7 +693,8 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen) static void child_handler(int signo) { - /* Otherwise empty handler because systemcalls will get interrupted + /* + * Otherwise empty handler because systemcalls will get interrupted * upon signal receipt * SysV needs the handler to be rearmed */
@@ -1089,8 +1078,7 @@ int main(int argc, char **argv) if (log_syslog) { openlog("git-daemon", LOG_PID, LOG_DAEMON); set_die_routine(daemon_die); - } - else + } else setlinebuf(stderr); /* avoid splitting a message in the middle */ if (inetd_mode && (group_name || user_name))
--
1.6.0.129.ge10d2