Skip to content

Commit 90dca67

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 61d3633 commit 90dca67

File tree

2 files changed

+14
-4
lines changed

2 files changed

+14
-4
lines changed

pack-write.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,19 @@ void fixup_pack_header_footer(int pack_fd,
213213
git_SHA_CTX old_sha1_ctx, new_sha1_ctx;
214214
struct pack_header hdr;
215215
char *buf;
216+
ssize_t read_result;
216217

217218
git_SHA1_Init(&old_sha1_ctx);
218219
git_SHA1_Init(&new_sha1_ctx);
219220

220221
if (lseek(pack_fd, 0, SEEK_SET) != 0)
221222
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)
223225
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);
224229
if (lseek(pack_fd, 0, SEEK_SET) != 0)
225230
die_errno("Failed seeking to start of '%s'", pack_name);
226231
git_SHA1_Update(&old_sha1_ctx, &hdr, sizeof(hdr));

sha1_file.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,10 +1757,15 @@ static int index_core(unsigned char *sha1, int fd, size_t size,
17571757
ret = index_mem(sha1, "", size, type, path, flags);
17581758
} else if (size <= SMALL_FILE_SIZE) {
17591759
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>");
17621767
else
1763-
ret = error_errno("short read");
1768+
ret = index_mem(sha1, buf, size, type, path, flags);
17641769
free(buf);
17651770
} else {
17661771
void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);

0 commit comments

Comments
 (0)