From 90dca6710e6e5aad5d78d0cd006c3adadb65524d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 27 Sep 2017 02:01:07 -0400 Subject: 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 Signed-off-by: Junio C Hamano --- pack-write.c | 7 ++++++- sha1_file.c | 11 ++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pack-write.c b/pack-write.c index a333ec6754..fea6284192 100644 --- 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 dd7cbe52ef..11346cf6d8 100644 --- 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 : ""); + else if (read_result != size) + ret = error("short read while indexing %s", + path ? path : ""); 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); -- cgit v1.2.3