Re: Confusing git messages when disk is full.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 15, 2017, 21:47 UTC
Message-ID: <xmqqk28r2kk4.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170215213221.lnraiktneokpk3mg@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> On Sun, Feb 12, 2017 at 05:37:30PM +0100, Jáchym Barvínek wrote:
> If FETCH_HEAD failed to write because of a full disk (or any other
> reason), then the right thing is for "git fetch" to write an error to
> stderr, and git-pull should not continue the operation at all.
>
> If we're not doing that, then that is certainly a bug.

One suspect would be store_updated_refs(). We do catch failure from fopen("a") of FETCH_HEAD (it is truncated earlier in the code when the --append option is not given), but all the writes go through stdio without error checking.

I wonder if this lazy patch is sufficient. I want to avoid having to sprinkle

	if (fputs("\\n", fp))
		error(...);

all over the code.

 builtin/fetch.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b5ad09d046..72347f0054 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,7 +868,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
  abort:
 	strbuf_release(&note);
 	free(url);
-	fclose(fp);
+	if (ferror(fp))
+		rc = -1;
+	if (fclose(fp))
+		rc = -1;
 	return rc;
 }