Re: Confusing git messages when disk is full.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 15, 2017, 22:50 UTC
Message-ID: <xmqq37ff2hn8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170215223246.mkaz22yrovnscnne@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 13 quoted lines

> Good catch. I think we use a nasty bitwise-OR elsewhere to do that.
> Ah, here it is, in tempfile.c:
>
>                 /*
>                  * Note: no short-circuiting here; we want to fclose()
>                  * in any case!
>                  */
>                 err = ferror(fp) | fclose(fp);
>
> That works, but the fact that we need a comment is a good sign that it's
> kind of gross. It's too bad stdio does not specify the return of fclose
> to report an error in the close _or_ any previous error. I guess we
> could wrap it with our own function.

Sure.  I am happy to add something like this:

	/*
	 * closes a FILE *, returns 0 if closing and all the
	 * previous stdio operations on fp were successful,
	 * otherwise non-zero.
	 */
	int xfclose(FILE *fp)
	{
		return ferror(fp) | fclose(fp);
	}

I do not think we should try to do anything fancier to allow the caller to tell ferror() and fclose() apart, as such a caller would then need to do

	switch (xfclose(fp)) {
	case 0: /* happy */ break;
	case XFCLOSE_CLOSE: do "close failed" thing; break;
	case XFCLOSE_ERROR: do "other things failed" thing; break;
	}

and at that point, "other things failed" code would not have much to work with to do more detailed diagnosis anyway (the errno is likely not trustable), and it is not too much to write

	if (ferror(fp))
		do "saw some failure before" thing;
	if (fclose(fp))
		do "close failed" thing;

instead.