From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 14 Jul 2013 21:48:12 GMT
Subject: Re: [PATCH 1/4] daemon.c:handle: Remove unneeded check for null pointer.
Message-ID: <20130714214812.GA13444@google.com>
In-Reply-To: <1373837749-14402-1-git-send-email-stefanbeller@googlemail.com>

Hi,

Stefan Beller wrote:

> addr doesn't need to be checked at that line as it it already accessed
> 7 lines before in the if (addr->sa_family).

Good catch.  This asymmetry has been present since the lines were first
introduced (all guarded by "if (addr)") in v1.4.1-rc1~3^2~4 (Log peer
address when git-daemon called from inetd, 2006-06-20).

> --- a/daemon.c
> +++ b/daemon.c
> @@ -754,19 +754,19 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
>  	}
>  
>  	if (addr->sa_family == AF_INET) {
>  		struct sockaddr_in *sin_addr = (void *) addr;
>  		inet_ntop(addr->sa_family, &sin_addr->sin_addr, addrbuf + 12,
>  		    sizeof(addrbuf) - 12);
>  		snprintf(portbuf, sizeof(portbuf), "REMOTE_PORT=%d",
>  		    ntohs(sin_addr->sin_port));
>  #ifndef NO_IPV6
> -	} else if (addr && addr->sa_family == AF_INET6) {
> +	} else if (addr->sa_family == AF_INET6) {

At this point 'addr' is &ss.sa from service_loop, so it really cannot
be NULL.

So fwiw, I like this patch.