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

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

[Alex Riesen]

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

[PATCH 2/3] Make chdir failures visible

[Alex Riesen]

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 git.c   |    4 ++--
 setup.c |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/git.c b/git.c
index 9e5813c..940a498 100644
--- a/git.c
+++ b/git.c
@@ -195,8 +195,8 @@ static int handle_alias(int *argcp, const char ***argv)
 		ret = 1;
 	}
 
-	if (subdir)
-		chdir(subdir);
+	if (subdir && chdir(subdir))
+		die("Cannot change to %s: %s", subdir, strerror(errno));
 
 	errno = saved_errno;
 
diff --git a/setup.c b/setup.c
index 78a8041..833ced2 100644
--- a/setup.c
+++ b/setup.c
@@ -470,7 +470,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			}
 			die("Not a git repository");
 		}
-		chdir("..");
+		if (chdir(".."))
+			die("Cannot change to %s/..: %s", cwd, strerror(errno));
 	}
 
 	inside_git_dir = 0;
-- 
1.6.1.rc1.29.gb140

[PATCH 3/3] Report symlink failures in merge-recursive

[Alex Riesen]

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

I don't check for unlink success in the line above, because symlink
will fail if unlink failed to cleanup the new path.

 merge-recursive.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 0e988f2..a0c804c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -525,7 +525,8 @@ static void update_file_flags(struct merge_options *o,
 			char *lnk = xmemdupz(buf, size);
 			safe_create_leading_directories_const(path);
 			unlink(path);
-			symlink(lnk, path);
+			if (symlink(lnk, path))
+				die("failed to symlink %s: %s", path, strerror(errno));
 			free(lnk);
 		} else
 			die("do not know what to do with %06o %s '%s'",
-- 
1.6.1.rc1.29.gb140

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

[Junio C Hamano]

All of them obviously look correct.  Will apply to the tip of master.

Thanks.