detect close failure on just-written file handles

meyering · gitster · commit 91c8d5905c33 · 2007-06-26T21:48:53.000-07:00
I audited git for potential undetected write failures.
In the cases fixed below, the diagnostics I add mimic the diagnostics
used in surrounding code, even when that means not reporting
the precise strerror(errno) cause of the error.

Signed-off-by: Jim Meyering &lt;jim@meyering.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin-init-db.c b/builtin-init-db.c
@@ -40,7 +40,8 @@ static int copy_file(const char *dst, const char *src, int mode)
 		return fdo;
 	}
 	status = copy_fd(fdi, fdo);
-	close(fdo);
+	if (close(fdo) != 0)
+		return error("%s: write error: %s", dst, strerror(errno));
 
 	if (!status && adjust_shared_perm(dst))
 		return -1;
diff --git a/builtin-rerere.c b/builtin-rerere.c
@@ -57,7 +57,8 @@ static int write_rr(struct path_list *rr, int out_fd)
 		    write_in_full(out_fd, path, length) != length)
 			die("unable to write rerere record");
 	}
-	close(out_fd);
+	if (close(out_fd) != 0)
+		die("unable to write rerere record");
 	return commit_lock_file(&write_lock);
 }
 
diff --git a/index-pack.c b/index-pack.c
@@ -634,7 +634,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 				write_or_die(keep_fd, keep_msg, keep_msg_len);
 				write_or_die(keep_fd, "\n", 1);
 			}
-			close(keep_fd);
+			if (close(keep_fd) != 0)
+				die("cannot write keep file");
 			report = "keep";
 		}
 	}
diff --git a/refs.c b/refs.c
@@ -1106,8 +1106,7 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 		len += sprintf(logrec + len - 1, "\t%.*s\n", msglen, msg) - 1;
 	written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
 	free(logrec);
-	close(logfd);
-	if (written != len)
+	if (close(logfd) != 0 || written != len)
 		return error("Unable to append to %s", log_file);
 	return 0;
 }
@@ -1204,8 +1203,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 		goto error_free_return;
 	}
 	written = write_in_full(fd, ref, len);
-	close(fd);
-	if (written != len) {
+	if (close(fd) != 0 || written != len) {
 		error("Unable to write to %s", lockpath);
 		goto error_unlink_return;
 	}

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,8 @@ static int copy_file(const char dst, const char src, int mode)`
`40`	`40`	`return fdo;`
`41`	`41`	`}`
`42`	`42`	`status = copy_fd(fdi, fdo);`
`43`		`- close(fdo);`
	`43`	`+ if (close(fdo) != 0)`
	`44`	`+ return error("%s: write error: %s", dst, strerror(errno));`
`44`	`45`
`45`	`46`	`if (!status && adjust_shared_perm(dst))`
`46`	`47`	`return -1;`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,8 @@ static int write_rr(struct path_list *rr, int out_fd)`
`57`	`57`	`write_in_full(out_fd, path, length) != length)`
`58`	`58`	`die("unable to write rerere record");`
`59`	`59`	`}`
`60`		`- close(out_fd);`
	`60`	`+ if (close(out_fd) != 0)`
	`61`	`+ die("unable to write rerere record");`
`61`	`62`	`return commit_lock_file(&write_lock);`
`62`	`63`	`}`
`63`	`64`
Original file line number	Diff line number	Diff line change
`@@ -634,7 +634,8 @@ static void final(const char final_pack_name, const char curr_pack_name,`
`634`	`634`	`write_or_die(keep_fd, keep_msg, keep_msg_len);`
`635`	`635`	`write_or_die(keep_fd, "\n", 1);`
`636`	`636`	`}`
`637`		`- close(keep_fd);`
	`637`	`+ if (close(keep_fd) != 0)`
	`638`	`+ die("cannot write keep file");`
`638`	`639`	`report = "keep";`
`639`	`640`	`}`
`640`	`641`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1106,8 +1106,7 @@ static int log_ref_write(const char ref_name, const unsigned char old_sha1,`
`1106`	`1106`	`len += sprintf(logrec + len - 1, "\t%.*s\n", msglen, msg) - 1;`
`1107`	`1107`	`written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;`
`1108`	`1108`	`free(logrec);`
`1109`		`- close(logfd);`
`1110`		`- if (written != len)`
	`1109`	`+ if (close(logfd) != 0 \|\| written != len)`
`1111`	`1110`	`return error("Unable to append to %s", log_file);`
`1112`	`1111`	`return 0;`
`1113`	`1112`	`}`
`@@ -1204,8 +1203,7 @@ int create_symref(const char ref_target, const char refs_heads_master,`
`1204`	`1203`	`goto error_free_return;`
`1205`	`1204`	`}`
`1206`	`1205`	`written = write_in_full(fd, ref, len);`
`1207`		`- close(fd);`
`1208`		`- if (written != len) {`
	`1206`	`+ if (close(fd) != 0 \|\| written != len) {`
`1209`	`1207`	`error("Unable to write to %s", lockpath);`
`1210`	`1208`	`goto error_unlink_return;`
`1211`	`1209`	`}`