From: Jeff King <peff@peff.net>
Date: Tue, 16 Sep 2014 00:28:25 GMT
Subject: Re: [PATCH] credential-cache: close stderr in daemon process
Message-ID: <20140916002825.GD5019@peff.net>
In-Reply-To: <xmqqoaug4la4.fsf@gitster.dls.corp.google.com>

On Mon, Sep 15, 2014 at 02:38:11PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > +	if (!debug)
> > +		freopen("/dev/null", "w", stderr);
> 
> I am getting this:
> 
> credential-cache--daemon.c:216:10: error: ignoring return value of
> 'freopen', declared with attribute warn_unused_result
> [-Werror=unused-result]

Hmph, my glibc does not seem to mark freopen. It's probably sane to
check it for error, as we would otherwise probably segfault on the next
call to "fprintf(stderr)".

>From my reading of freopen(3posix), I do not think we need to do
anything like "stderr = freopen(..., stderr);". The filehandle is
repointed, not re-allocated. And anyway, that is the assumption all of
our other freopen calls make. :)

> which is somewhat irritating.  Even though I am not irritated by
> this code, but by the compiler and glibc headers, this is apparently
> the only offending one, so we may want to fix it anyway.

Squash this in?

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index c07a67c..c2f0049 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -212,8 +212,10 @@ static void serve_cache(const char *socket_path, int debug)
 
 	printf("ok\n");
 	fclose(stdout);
-	if (!debug)
-		freopen("/dev/null", "w", stderr);
+	if (!debug) {
+		if (!freopen("/dev/null", "w", stderr))
+			die_errno("unable to point stderr to /dev/null");
+	}
 
 	while (serve_cache_loop(fd))
 		; /* nothing */