All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Jáchym Barvínek" <jachymb@gmail.com>, git@vger.kernel.org
Subject: Re: Confusing git messages when disk is full.
Date: Wed, 15 Feb 2017 14:50:19 -0800	[thread overview]
Message-ID: <xmqq37ff2hn8.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170215223246.mkaz22yrovnscnne@sigill.intra.peff.net> (Jeff King's message of "Wed, 15 Feb 2017 17:32:46 -0500")

Jeff King <peff@peff.net> writes:

> 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.
        

  reply	other threads:[~2017-02-15 22:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-12 16:37 Confusing git messages when disk is full Jáchym Barvínek
2017-02-15 21:32 ` Jeff King
2017-02-15 21:47   ` Junio C Hamano
2017-02-15 21:51     ` Jeff King
2017-02-15 22:28       ` Junio C Hamano
2017-02-15 22:32         ` Jeff King
2017-02-15 22:50           ` Junio C Hamano [this message]
2017-02-15 23:18             ` Jeff King
2017-02-16 10:10               ` Andreas Schwab
2017-02-16 16:44                 ` Jeff King
2017-02-16 21:31                   ` [PATCH] tempfile: avoid "ferror | fclose" trick Jeff King
2017-02-17  8:00                     ` Michael Haggerty
2017-02-17  8:07                       ` Jeff King
2017-02-17 10:42                         ` Michael Haggerty
2017-02-17 20:54                           ` Jeff King
2017-02-17 21:07                             ` Jeff King
2017-02-17 21:17                             ` Junio C Hamano
2017-02-17 21:21                               ` Jeff King
2017-02-17 21:42                                 ` Junio C Hamano
2017-02-17 22:10                                   ` Jeff King
2017-02-17 22:40                                     ` Junio C Hamano
2017-02-17 23:39                                       ` Jeff King
2017-02-17 23:52                                         ` Junio C Hamano
2017-02-17 23:54                                           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq37ff2hn8.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jachymb@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.