avoid looking at errno for short read_in_full() returns

When a caller tries to read a particular set of bytes via
read_in_full(), there are three possible outcomes:

1. An error, in which case -1 is returned and errno is
set.

2. A short read, in which fewer bytes are returned and
errno is unspecified (we never saw a read error, so we
may have some random value from whatever syscall failed
last).

3. The full read completed successfully.

Many callers handle cases 1 and 2 together by just checking
the result against the requested size. If their combined
error path looks at errno (e.g., by calling die_errno), they
may report a nonsense value.

Let's fix these sites by having them distinguish between the
two error cases. That avoids the random errno confusion, and
lets us give more detailed error messages.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/pack-write.c b/pack-write.c
index a333ec675476b4f10a5e12fe3303333ea200faea..fea62841920c9647edfcfba249a59bfb91170d8e 100644 (file)
--- a/pack-write.c
+++ b/pack-write.c
@@ -213,14 +213,19 @@ void fixup_pack_header_footer(int pack_fd,
        git_SHA_CTX old_sha1_ctx, new_sha1_ctx;
        struct pack_header hdr;
        char *buf;
+       ssize_t read_result;
 
        git_SHA1_Init(&old_sha1_ctx);
        git_SHA1_Init(&new_sha1_ctx);
 
        if (lseek(pack_fd, 0, SEEK_SET) != 0)
                die_errno("Failed seeking to start of '%s'", pack_name);
-       if (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
+       read_result = read_in_full(pack_fd, &hdr, sizeof(hdr));
+       if (read_result < 0)
                die_errno("Unable to reread header of '%s'", pack_name);
+       else if (read_result != sizeof(hdr))
+               die_errno("Unexpected short read for header of '%s'",
+                         pack_name);
        if (lseek(pack_fd, 0, SEEK_SET) != 0)
                die_errno("Failed seeking to start of '%s'", pack_name);
        git_SHA1_Update(&old_sha1_ctx, &hdr, sizeof(hdr));

diff --git a/sha1_file.c b/sha1_file.c
index dd7cbe52ef73754f30dfc61379549bc2fbd5e86c..11346cf6d83e69cfb39ab059de8cae6f6c982c64 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1757,10 +1757,15 @@ static int index_core(unsigned char *sha1, int fd, size_t size,
                ret = index_mem(sha1, "", size, type, path, flags);
        } else if (size <= SMALL_FILE_SIZE) {
                char *buf = xmalloc(size);
-               if (size == read_in_full(fd, buf, size))
-                       ret = index_mem(sha1, buf, size, type, path, flags);
+               ssize_t read_result = read_in_full(fd, buf, size);
+               if (read_result < 0)
+                       ret = error_errno("read error while indexing %s",
+                                         path ? path : "<unknown>");
+               else if (read_result != size)
+                       ret = error("short read while indexing %s",
+                                   path ? path : "<unknown>");
                else
-                       ret = error_errno("short read");
+                       ret = index_mem(sha1, buf, size, type, path, flags);
                free(buf);
        } else {
                void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);