git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] Make some of fwrite/fclose/write/close failures visible

From
Alex Riesen <raa.lkml@gmail.com>
Date
Dec 5, 2008, 00:35 UTC
Message-ID
<20081205003546.GA7294@blimp.localdomain>

So that full filesystem conditions or permissions problems wont go unnoticed.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

This and the follow-up patches is fallout of Windows debugging sessions. I implemented random error handling code just to figure out where it might be going wrong. None of that code actually helped to fix something (and lots was just thrown away), but some, I think, still makes sense.

This patch adds error handling only to fwrite/fputs where the return value was ignored and writing was definitely into a file. BTW, libc headers in Ubuntu 8.10 have warn_unused_result attribute added to many functions (fwrite(3) and write(2) amongst them). This helps finding the problem places.

 builtin-fsck.c  |    8 ++++++--
 builtin-merge.c |    6 ++++--
 rerere.c        |   46 +++++++++++++++++++++++++++++++++++-----------
 3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/builtin-fsck.c b/builtin-fsck.c
index d3f3de9..afded5e 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -201,12 +201,16 @@ static void check_unreachable_object(struct object *obj)
 				char *buf = read_sha1_file(obj->sha1,
 						&type, &size);
 				if (buf) {
-					fwrite(buf, size, 1, f);
+					if (fwrite(buf, size, 1, f) != 1)
+						die("Could not write %s: %s",
+						    filename, strerror(errno));
 					free(buf);
 				}
 			} else
 				fprintf(f, "%s\n", sha1_to_hex(obj->sha1));
-			fclose(f);
+			if (fclose(f))
+				die("Could not finish %s: %s",
+				    filename, strerror(errno));
 		}
 		return;
 	}
diff --git a/builtin-merge.c b/builtin-merge.c
index 7c2b90c..cf86975 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -293,8 +293,10 @@ static void squash_message(void)
 		pretty_print_commit(rev.commit_format, commit, &out, rev.abbrev,
 			NULL, NULL, rev.date_mode, 0);
 	}
-	write(fd, out.buf, out.len);
-	close(fd);
+	if (write(fd, out.buf, out.len) < 0)
+		die("Writing SQUASH_MSG: %s", strerror(errno));
+	if (close(fd))
+		die("Finishing SQUASH_MSG: %s", strerror(errno));
 	strbuf_release(&out);
 }
 
diff --git a/rerere.c b/rerere.c
index 02931a1..718fb52 100644
--- a/rerere.c
+++ b/rerere.c
@@ -70,6 +70,19 @@ static int write_rr(struct string_list *rr, int out_fd)
 	return 0;
 }
 
+static void ferr_write(const void *p, size_t count, FILE *fp, int *err)
+{
+	if (!count || *err)
+		return;
+	if (fwrite(p, count, 1, fp) != 1)
+		*err = errno;
+}
+
+static inline void ferr_puts(const char *s, FILE *fp, int *err)
+{
+	ferr_write(s, strlen(s), fp, err);
+}
+
 static int handle_file(const char *path,
 	 unsigned char *sha1, const char *output)
 {
@@ -82,6 +95,7 @@ static int handle_file(const char *path,
 	struct strbuf one = STRBUF_INIT, two = STRBUF_INIT;
 	FILE *f = fopen(path, "r");
 	FILE *out = NULL;
+	int wrerror = 0;
 
 	if (!f)
 		return error("Could not open %s", path);
@@ -118,11 +132,11 @@ static int handle_file(const char *path,
 			hunk_no++;
 			hunk = RR_CONTEXT;
 			if (out) {
-				fputs("<<<<<<<\n", out);
-				fwrite(one.buf, one.len, 1, out);
-				fputs("=======\n", out);
-				fwrite(two.buf, two.len, 1, out);
-				fputs(">>>>>>>\n", out);
+				ferr_puts("<<<<<<<\n", out, &wrerror);
+				ferr_write(one.buf, one.len, out, &wrerror);
+				ferr_puts("=======\n", out, &wrerror);
+				ferr_write(two.buf, two.len, out, &wrerror);
+				ferr_puts(">>>>>>>\n", out, &wrerror);
 			}
 			if (sha1) {
 				git_SHA1_Update(&ctx, one.buf ? one.buf : "",
@@ -139,7 +153,7 @@ static int handle_file(const char *path,
 		else if (hunk == RR_SIDE_2)
 			strbuf_addstr(&two, buf);
 		else if (out)
-			fputs(buf, out);
+			ferr_puts(buf, out, &wrerror);
 		continue;
 	bad:
 		hunk = 99; /* force error exit */
@@ -149,8 +163,12 @@ static int handle_file(const char *path,
 	strbuf_release(&two);
 
 	fclose(f);
-	if (out)
-		fclose(out);
+	if (wrerror)
+		error("There were errors while writing %s (%s)",
+		      path, strerror(wrerror));
+	if (out && fclose(out))
+		wrerror = error("Failed to flush %s: %s",
+				path, strerror(errno));
 	if (sha1)
 		git_SHA1_Final(sha1, &ctx);
 	if (hunk != RR_CONTEXT) {
@@ -158,6 +176,8 @@ static int handle_file(const char *path,
 			unlink(output);
 		return error("Could not parse conflict hunks in %s", path);
 	}
+	if (wrerror)
+		return -1;
 	return hunk_no;
 }
 
@@ -200,9 +220,13 @@ static int merge(const char *name, const char *path)
 	if (!ret) {
 		FILE *f = fopen(path, "w");
 		if (!f)
-			return error("Could not write to %s", path);
-		fwrite(result.ptr, result.size, 1, f);
-		fclose(f);
+			return error("Could not open %s: %s", path,
+				     strerror(errno));
+		if (fwrite(result.ptr, result.size, 1, f) != 1)
+			error("Could not write %s: %s", path, strerror(errno));
+		if (fclose(f))
+			return error("Writing %s failed: %s", path,
+				     strerror(errno));
 	}
 
 	free(cur.ptr);
-- 
1.6.1.rc1.29.gb140
Next: Alex Riesen
Message 1 of 4 in “Make some of fwrite/fclose/write/close failures visible”
  1. 1/3 Make some of fwrite/fclose/write/close failures visibleAlex Riesen, Dec 5, 2008
  2. 2/3 Make chdir failures visibleAlex Riesen, Dec 5, 2008
  3. 3/3 Report symlink failures in merge-recursiveAlex Riesen, Dec 5, 2008
  4. Junio C HamanoDec 5, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.