@matheusbsbs / matheus-bagliano / commits / 90dca6710e

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>

Jeff King committed Sep 27, 2017 at 02:01 UTC 90dca6710e6e5aad5d78d0cd006c3adadb65524d

2 files changed +14 -4

pack-write.c

+6 -1

@@ -213,14 +213,19 @@ void fixup_pack_header_footer(int pack_fd,

213 git_SHA_CTX old_sha1_ctx, new_sha1_ctx;

214 struct pack_header hdr;

215 char *buf;

216 + ssize_t read_result;

217

218 git_SHA1_Init(&old_sha1_ctx);

219 git_SHA1_Init(&new_sha1_ctx);

220

221 if (lseek(pack_fd, 0, SEEK_SET) != 0)

222 die_errno("Failed seeking to start of '%s'", pack_name);

222 - if (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))

223 + read_result = read_in_full(pack_fd, &hdr, sizeof(hdr));

224 + if (read_result < 0)

225 die_errno("Unable to reread header of '%s'", pack_name);

226 + else if (read_result != sizeof(hdr))

227 + die_errno("Unexpected short read for header of '%s'",

228 + pack_name);

229 if (lseek(pack_fd, 0, SEEK_SET) != 0)

230 die_errno("Failed seeking to start of '%s'", pack_name);

231 git_SHA1_Update(&old_sha1_ctx, &hdr, sizeof(hdr));

sha1_file.c

+8 -3

@@ -1757,10 +1757,15 @@ static int index_core(unsigned char *sha1, int fd, size_t size,

1757 ret = index_mem(sha1, "", size, type, path, flags);

1758 } else if (size <= SMALL_FILE_SIZE) {

1759 char *buf = xmalloc(size);

1760 - if (size == read_in_full(fd, buf, size))

1761 - ret = index_mem(sha1, buf, size, type, path, flags);

1760 + ssize_t read_result = read_in_full(fd, buf, size);

1761 + if (read_result < 0)

1762 + ret = error_errno("read error while indexing %s",

1763 + path ? path : "<unknown>");

1764 + else if (read_result != size)

1765 + ret = error("short read while indexing %s",

1766 + path ? path : "<unknown>");

1767 else

1763 - ret = error_errno("short read");

1768 + ret = index_mem(sha1, buf, size, type, path, flags);

1769 free(buf);

1770 } else {

1771 void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);