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@xxxxxxxx>
---
This shouldn't change any behaviour, and it's obviously touching some core
code, so maybe it's not worth it. On the other hand, from a longer-term
maintenance standpoint and from a "be much more careful when doing file
writes" standpoint, I think it's worth it.
The re-write is "obviously correct" (famous last words) and is mostly
just moving code around and getting rid of a few temporaries that become
unnecessary as a result.
The patch looks a bit messy: the changes aren't actually that big, but the
split-up and the resulting re-indentation makes the patch fairly
unreadable, so the cleanups are more obvious when you look at the
before-and-after side by side rather than when looking at the unified
diff..)
diff --git a/sha1_file.c b/sha1_file.c
index 2230010..c2fe7c2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1399,6 +1399,25 @@ int move_temp_to_file(const char *tmpfil
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
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
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);
+ /* Set it up */
+ memset(&stream, 0, sizeof(stream));
+ deflateInit(&stream, Z_BEST_COMPRESSION);
+ size = deflateBound(&stream, len + hdrlen);
+ buf = xmalloc(size);
- /* Compress it */
- stream.next_out = buf;
- stream.avail_out = 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 */;
+ /* First header.. */
+ stream.next_in = (void *)hdr;
+ stream.avail_in = hdrlen;
+ while (deflate(&stream, 0) == Z_OK)
+ /* nothing */;
- /* 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;
- }
+ /* Then the data itself.. */
+ stream.next_in = unpacked;
+ stream.avail_in = len;
+ while (deflate(&stream, Z_FINISH) == Z_OK)
+ /* nothing */;
+ deflateEnd(&stream);
+ free(unpacked);
- 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);
+ *objsize = stream.total_out;
+ return buf;
+}
- if (map)
- munmap(map, objsize);
- if (temp_obj)
- free(temp_obj);
+int write_sha1_to_fd(int fd, const unsigned char *sha1)
+{
+ int retval;
+ unsigned long objsize;
+ void *buf = map_sha1_file_internal(sha1, &objsize);
- return 0;
+ 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 ch
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;
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html