copy_fd: pass error message back through a strbuf

This way, callers can put the message in context or even avoid
printing the message altogether.

Currently hold_lock_file_for_append tries to save errno in order to
produce a meaningful message about the failure and tries to print a
second message using errno.  Unfortunately the errno it uses is not
meaningful because copy_fd makes no effort to set a meaningful errno
value.  This change is preparation for simplifying the
hold_lock_file_for_append behavior.

No user-visible change intended yet.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

4 files changed, 32 insertions, 14 deletions

diff --git a/cache.h b/cache.h
index 99ed096aed..ddaa30f360 100644
--- a/cache.h
+++ b/cache.h
@@ -1479,7 +1479,7 @@ extern const char *git_mailmap_blob;
 extern void maybe_flush_or_die(FILE *, const char *);
 __attribute__((format (printf, 2, 3)))
 extern void fprintf_or_die(FILE *, const char *fmt, ...);
-extern int copy_fd(int ifd, int ofd);
+extern int copy_fd(int ifd, int ofd, struct strbuf *err);
 extern int copy_file(const char *dst, const char *src, int mode);
 extern int copy_file_with_time(const char *dst, const char *src, int mode);
 extern void write_or_die(int fd, const void *buf, size_t count);
diff --git a/convert.c b/convert.c
index 9a5612e93d..e301447abd 100644
--- a/convert.c
+++ b/convert.c
@@ -358,7 +358,11 @@ static int filter_buffer_or_fd(int in, int out, void *data)
 	if (params->src) {
 		write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
 	} else {
-		write_err = copy_fd(params->fd, child_process.in);
+		struct strbuf err = STRBUF_INIT;
+		write_err = copy_fd(params->fd, child_process.in, &err);
+		if (write_err)
+			error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
 	}
 
 	if (close(child_process.in))
diff --git a/copy.c b/copy.c
index f2970ec462..828661aca7 100644
--- a/copy.c
+++ b/copy.c
@@ -1,19 +1,22 @@
 #include "cache.h"
 
-int copy_fd(int ifd, int ofd)
+int copy_fd(int ifd, int ofd, struct strbuf *err)
 {
+	assert(err);
+
 	while (1) {
 		char buffer[8192];
 		ssize_t len = xread(ifd, buffer, sizeof(buffer));
 		if (!len)
 			break;
 		if (len < 0) {
-			return error("copy-fd: read returned %s",
-				     strerror(errno));
+			strbuf_addf(err, "read returned %s", strerror(errno));
+			return -1;
+		}
+		if (write_in_full(ofd, buffer, len) < 0) {
+			strbuf_addf(err, "write returned %s", strerror(errno));
+			return -1;
 		}
-		if (write_in_full(ofd, buffer, len) < 0)
-			return error("copy-fd: write returned %s",
-				     strerror(errno));
 	}
 	return 0;
 }
@@ -33,7 +36,8 @@ static int copy_times(const char *dst, const char *src)
 
 int copy_file(const char *dst, const char *src, int mode)
 {
-	int fdi, fdo, status;
+	int fdi, fdo;
+	struct strbuf err = STRBUF_INIT;
 
 	mode = (mode & 0111) ? 0777 : 0666;
 	if ((fdi = open(src, O_RDONLY)) < 0)
@@ -42,15 +46,21 @@ int copy_file(const char *dst, const char *src, int mode)
 		close(fdi);
 		return fdo;
 	}
-	status = copy_fd(fdi, fdo);
+	if (copy_fd(fdi, fdo, &err)) {
+		close(fdi);
+		close(fdo);
+		error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
+		return -1;
+	}
+	strbuf_release(&err);
 	close(fdi);
 	if (close(fdo) != 0)
 		return error("%s: close error: %s", dst, strerror(errno));
-
-	if (!status && adjust_shared_perm(dst))
+	if (adjust_shared_perm(dst))
 		return -1;
 
-	return status;
+	return 0;
 }
 
 int copy_file_with_time(const char *dst, const char *src, int mode)
diff --git a/lockfile.c b/lockfile.c
index 4f16ee78ce..b3020f3fae 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -182,6 +182,7 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 {
 	int fd, orig_fd;
+	struct strbuf err = STRBUF_INIT;
 
 	fd = lock_file(lk, path, flags);
 	if (fd < 0) {
@@ -202,9 +203,11 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 			errno = save_errno;
 			return -1;
 		}
-	} else if (copy_fd(orig_fd, fd)) {
+	} else if (copy_fd(orig_fd, fd, &err)) {
 		int save_errno = errno;
 
+		error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
 		if (flags & LOCK_DIE_ON_ERROR)
 			exit(128);
 		close(orig_fd);
@@ -214,6 +217,7 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 	} else {
 		close(orig_fd);
 	}
+	strbuf_release(&err);
 	return fd;
 }