copy_fd(): do not close the input file descriptor

sprohaska · gitster · commit b29763aa9bcb · 2014-08-28T10:25:14.000-07:00
The caller, not this function, opened the file descriptor; it is
selfish for the callee to close it when it is done reading from it.
The caller may want an option to rewind and re-read the contents
after it returns.

Simplify the loop to copy the input in full to the output; its
body essentially is what a call to write_in_full() helper does.

Signed-off-by: Steffen Prohaska &lt;prohaska@zib.de&gt;
Helped-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/copy.c b/copy.c
@@ -4,34 +4,17 @@ int copy_fd(int ifd, int ofd)
 {
 	while (1) {
 		char buffer[8192];
-		char *buf = buffer;
 		ssize_t len = xread(ifd, buffer, sizeof(buffer));
 		if (!len)
 			break;
 		if (len < 0) {
-			int read_error = errno;
-			close(ifd);
 			return error("copy-fd: read returned %s",
-				     strerror(read_error));
-		}
-		while (len) {
-			int written = xwrite(ofd, buf, len);
-			if (written > 0) {
-				buf += written;
-				len -= written;
-			}
-			else if (!written) {
-				close(ifd);
-				return error("copy-fd: write returned 0");
-			} else {
-				int write_error = errno;
-				close(ifd);
-				return error("copy-fd: write returned %s",
-					     strerror(write_error));
-			}
+				     strerror(errno));
 		}
+		if (write_in_full(ofd, buffer, len) < 0)
+			return error("copy-fd: write returned %s",
+				     strerror(errno));
 	}
-	close(ifd);
 	return 0;
 }
 
@@ -60,6 +43,7 @@ int copy_file(const char *dst, const char *src, int mode)
 		return fdo;
 	}
 	status = copy_fd(fdi, fdo);
+	close(fdi);
 	if (close(fdo) != 0)
 		return error("%s: close error: %s", dst, strerror(errno));
 
diff --git a/lockfile.c b/lockfile.c
@@ -224,8 +224,11 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 	} else if (copy_fd(orig_fd, fd)) {
 		if (flags & LOCK_DIE_ON_ERROR)
 			exit(128);
+		close(orig_fd);
 		close(fd);
 		return -1;
+	} else {
+		close(orig_fd);
 	}
 	return fd;
 }

Original file line number	Diff line number	Diff line change
`@@ -224,8 +224,11 @@ int hold_lock_file_for_append(struct lock_file lk, const char path, int flags)`
`224`	`224`	`} else if (copy_fd(orig_fd, fd)) {`
`225`	`225`	`if (flags & LOCK_DIE_ON_ERROR)`
`226`	`226`	`exit(128);`
	`227`	`+ close(orig_fd);`
`227`	`228`	`close(fd);`
`228`	`229`	`return -1;`
	`230`	`+ } else {`
	`231`	`+ close(orig_fd);`
`229`	`232`	`}`
`230`	`233`	`return fd;`
`231`	`234`	`}`