* "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 <gitster@xxxxxxxxx>:
---
Junio C Hamano <gitster@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
