Re: [PATCH] git-daemon: Simplify child management and associated logging by

From: Alex Riesen <raa.lkml@gmail.com>
Date: Aug 12, 2008, 22:32 UTC
Message-ID: <20080812223224.GA4134@steel.home>
In-Reply-To: <20080812212534.6871.19377.stgit@aristoteles.cuci.nl>

Stephen R. van den Berg, Tue, Aug 12, 2008 23:25:35 +0200:

> +        struct child*newborn;

You may want to reformat the patch using tabs instead of spaces.

> +        newborn = xmalloc(sizeof *newborn);

The custom here is to use "sizeof(type)". Brackets and typename.

> +        if (newborn) {
> +	        struct child**cradle,*blanket;

"struct child **cradle, *blanket;" (the spaces before asterisks)

> +		memcpy(memset(&newborn->address, 0, sizeof newborn->address),
> +		 addr, addrlen);

Aren't separate calls easier to read (and type)?

	memset(&newborn->address, 0, sizeof(newborn->address));
	memcpy(&newborn->address, addr, addrlen);

Show 12 quoted lines

> -static void kill_some_children(int signo, unsigned start, unsigned stop)
> +static void kill_some_child(int signo)
>  {
> -	start %= MAX_CHILDREN;
> -	stop %= MAX_CHILDREN;
> -	while (start != stop) {
> -		if (!(start & 3))
> -			kill(live_child[start].pid, signo);
> -		start = (start + 1) % MAX_CHILDREN;
> +	const struct child *blanket;
> +
> +	if ((blanket = firstborn)) {

	if (firstborn) {
	    const struct child *blanket = firstborn;

You don't even use blanket outside of the "if".

>  static void check_dead_children(void)
>  {
> +		loginfo("[%d] Disconnected%s", (int)pid, dead);

BTW, why do you need that pid_t->int cast?

Show 6 quoted lines

> @@ -1105,6 +1026,10 @@ int main(int argc, char **argv)
>  			init_timeout = atoi(arg+15);
>  			continue;
>  		}
> +		if (!prefixcmp(arg, "--max-connections=")) {
> +			max_connections = atoi(arg+18);

An error checking wouldn't go amiss. And it can't be done with atoi (consider strtol).