Thread (3 messages) flat view 3 messages, 2 authors, 2016-06-15

Re: [PATCH] sha1_file.c (write_sha1_from_fd): Detect close failure.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:01

Jim Meyering [off-list ref] writes:
quoted hunk
I stumbled across this in the context of the fchmod 0444 patch.
At first, I was going to unlink and call error like the two subsequent
tests do, but a failed write (above) provokes a "die", so I made
this do the same.  This is testing for a write failure, after all.

Signed-off-by: Jim Meyering <redacted>
---
 sha1_file.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 0897b94..42aef33 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2155,7 +2155,8 @@ int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
 	inflateEnd(&stream);

 	fchmod(local, 0444);
-	close(local);
+	if (close(local) != 0)
+		die("unable to write sha1 file");
 	SHA1_Final(real_sha1, &c);
 	if (ret != Z_STREAM_END) {
 		unlink(tmpfile);
--
1.5.1.rc1.51.gb08b
Hmph.  Not catching error from close() is wrong, so this is an
improvement, but it still leaves tmpfile on the filesystem,
doesn't it?

Looking at write_sha1_file(), which is in a sense more important
than this function, it is worse.  We should also detect error
from close(), nuke the temporary file and return an error there.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help