Re: Confusing git messages when disk is full.

On Wed, Feb 15, 2017 at 02:28:10PM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

quoted

  abort:
 	strbuf_release(&note);
 	free(url);
-	fclose(fp);
+	if (ferror(fp))
+		rc = -1;
+	if (fclose(fp))
+		rc = -1;
 	return rc;

Yeah, I think this works. Normally you'd want to flush before checking
ferror(), but since you detect errors from fclose, too, it should be
fine.

We probably should write something stderr, though. Maybe:

  if (ferror(fp) || fclose(fp))
	rc = error_errno("unable to write to %s", filename);

Yes, and somehow make sure we do fclose(fp) even when we have an
error already ;-)

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.

-Peff