Clean up sha1 file writing

This cleans up and future-proofs the sha1 file writing in sha1_file.c.

In particular, instead of doing a simple "write()" call and just verifying
that it succeeds (or - as in one place - just assuming it does), it uses
"write_buffer()" to write data to the file descriptor while correctly
checking for partial writes, EINTR etc.

It also splits up write_sha1_to_fd() to be a lot more readable: if we need
to re-create the compressed object, we do so in a separate helper
function, making the logic a whole lot more modular and obvious.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

1 files changed, 78 insertions, 61 deletions

diff --git a/sha1_file.c b/sha1_file.c
index e444d9d..f77c189 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1399,6 +1399,25 @@ int move_temp_to_file(const char *tmpfile, char *filename)
 	return 0;
 }
 
+static int write_buffer(int fd, const void *buf, size_t len)
+{
+	while (len) {
+		ssize_t size;
+
+		size = write(fd, buf, len);
+		if (!size)
+			return error("file write: disk full");
+		if (size < 0) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
+			return error("file write error (%s)", strerror(errno));
+		}
+		len -= size;
+		buf += size;
+	}
+	return 0;
+}
+
 int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
 {
 	int size;
@@ -1465,8 +1484,8 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
 	deflateEnd(&stream);
 	size = stream.total_out;
 
-	if (write(fd, compressed, size) != size)
-		die("unable to write file");
+	if (write_buffer(fd, compressed, size) < 0)
+		die("unable to write sha1 file");
 	fchmod(fd, 0444);
 	close(fd);
 	free(compressed);
@@ -1474,73 +1493,70 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
 	return move_temp_to_file(tmpfile, filename);
 }
 
-int write_sha1_to_fd(int fd, const unsigned char *sha1)
+/*
+ * We need to unpack and recompress the object for writing
+ * it out to a different file.
+ */
+static void *repack_object(const unsigned char *sha1, unsigned long *objsize)
 {
-	ssize_t size;
-	unsigned long objsize;
-	int posn = 0;
-	void *map = map_sha1_file_internal(sha1, &objsize);
-	void *buf = map;
-	void *temp_obj = NULL;
+	size_t size;
 	z_stream stream;
+	unsigned char *unpacked;
+	unsigned long len;
+	char type[20];
+	char hdr[50];
+	int hdrlen;
+	void *buf;
 
-	if (!buf) {
-		unsigned char *unpacked;
-		unsigned long len;
-		char type[20];
-		char hdr[50];
-		int hdrlen;
-		// need to unpack and recompress it by itself
-		unpacked = read_packed_sha1(sha1, type, &len);
+	// need to unpack and recompress it by itself
+	unpacked = read_packed_sha1(sha1, type, &len);
 
-		hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
+	hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
 
-		/* Set it up */
-		memset(&stream, 0, sizeof(stream));
-		deflateInit(&stream, Z_BEST_COMPRESSION);
-		size = deflateBound(&stream, len + hdrlen);
-		temp_obj = buf = xmalloc(size);
-
-		/* Compress it */
-		stream.next_out = buf;
-		stream.avail_out = size;
-		
-		/* First header.. */
-		stream.next_in = (void *)hdr;
-		stream.avail_in = hdrlen;
-		while (deflate(&stream, 0) == Z_OK)
-			/* nothing */;
+	/* Set it up */
+	memset(&stream, 0, sizeof(stream));
+	deflateInit(&stream, Z_BEST_COMPRESSION);
+	size = deflateBound(&stream, len + hdrlen);
+	buf = xmalloc(size);
 
-		/* Then the data itself.. */
-		stream.next_in = unpacked;
-		stream.avail_in = len;
-		while (deflate(&stream, Z_FINISH) == Z_OK)
-			/* nothing */;
-		deflateEnd(&stream);
-		free(unpacked);
-		
-		objsize = stream.total_out;
-	}
+	/* Compress it */
+	stream.next_out = buf;
+	stream.avail_out = size;
 
-	do {
-		size = write(fd, buf + posn, objsize - posn);
-		if (size <= 0) {
-			if (!size) {
-				fprintf(stderr, "write closed\n");
-			} else {
-				perror("write ");
-			}
-			return -1;
-		}
-		posn += size;
-	} while (posn < objsize);
+	/* First header.. */
+	stream.next_in = (void *)hdr;
+	stream.avail_in = hdrlen;
+	while (deflate(&stream, 0) == Z_OK)
+		/* nothing */;
 
-	if (map)
-		munmap(map, objsize);
-	if (temp_obj)
-		free(temp_obj);
+	/* Then the data itself.. */
+	stream.next_in = unpacked;
+	stream.avail_in = len;
+	while (deflate(&stream, Z_FINISH) == Z_OK)
+		/* nothing */;
+	deflateEnd(&stream);
+	free(unpacked);
 
-	return 0;
+	*objsize = stream.total_out;
+	return buf;
+}
+
+int write_sha1_to_fd(int fd, const unsigned char *sha1)
+{
+	int retval;
+	unsigned long objsize;
+	void *buf = map_sha1_file_internal(sha1, &objsize);
+
+	if (buf) {
+		retval = write_buffer(fd, buf, objsize);
+		munmap(buf, objsize);
+		return retval;
+	}
+
+	buf = repack_object(sha1, &objsize);
+	retval = write_buffer(fd, buf, objsize);
+	free(buf);
+	return retval;
 }
 
 int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
@@ -1579,7 +1595,8 @@ int write_sha1_from_fd(const unsigned char *sha1, int fd, char *buffer,
 				SHA1_Update(&c, discard, sizeof(discard) -
 					    stream.avail_out);
 			} while (stream.avail_in && ret == Z_OK);
-			write(local, buffer, *bufposn - stream.avail_in);
+			if (write_buffer(local, buffer, *bufposn - stream.avail_in) < 0)
+				die("unable to write sha1 file");
 			memmove(buffer, buffer + *bufposn - stream.avail_in,
 				stream.avail_in);
 			*bufposn = stream.avail_in;