diff options
author | Junio C Hamano <gitster@pobox.com> | 2017-10-03 15:42:49 +0900 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2017-10-03 15:42:49 +0900 |
commit | cb1083ca23c2f78140b66b925ed0b82fe400eea5 (patch) | |
tree | c78509d2b9fbe79c99ed1acb83bf43886b4d0c0c | |
parent | Merge branch 'jk/no-optional-locks' (diff) | |
parent | worktree: check the result of read_in_full() (diff) | |
download | tgif-cb1083ca23c2f78140b66b925ed0b82fe400eea5.tar.xz |
Merge branch 'jk/read-in-full'
Code clean-up to prevent future mistakes by copying and pasting
code that checks the result of read_in_full() function.
* jk/read-in-full:
worktree: check the result of read_in_full()
worktree: use xsize_t to access file size
distinguish error versus short read from read_in_full()
avoid looking at errno for short read_in_full() returns
prefer "!=" when checking read_in_full() result
notes-merge: drop dead zero-write code
files-backend: prefer "0" for write_in_full() error check
-rw-r--r-- | builtin/get-tar-commit-id.c | 6 | ||||
-rw-r--r-- | builtin/worktree.c | 24 | ||||
-rw-r--r-- | bulk-checkin.c | 5 | ||||
-rw-r--r-- | csum-file.c | 2 | ||||
-rw-r--r-- | notes-merge.c | 2 | ||||
-rw-r--r-- | pack-write.c | 7 | ||||
-rw-r--r-- | packfile.c | 11 | ||||
-rw-r--r-- | pkt-line.c | 2 | ||||
-rw-r--r-- | refs/files-backend.c | 2 | ||||
-rw-r--r-- | sha1_file.c | 11 |
10 files changed, 55 insertions, 17 deletions
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c index 6d9a79f9b3..2706fcfaf2 100644 --- a/builtin/get-tar-commit-id.c +++ b/builtin/get-tar-commit-id.c @@ -26,8 +26,10 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix) usage(builtin_get_tar_commit_id_usage); n = read_in_full(0, buffer, HEADERSIZE); - if (n < HEADERSIZE) - die("git get-tar-commit-id: read error"); + if (n < 0) + die_errno("git get-tar-commit-id: read error"); + if (n != HEADERSIZE) + die_errno("git get-tar-commit-id: EOF before reading tar header"); if (header->typeflag[0] != 'g') return 1; if (!skip_prefix(content, "52 comment=", &comment)) diff --git a/builtin/worktree.c b/builtin/worktree.c index de26849f55..7b9307aa58 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -38,7 +38,9 @@ static int prune_worktree(const char *id, struct strbuf *reason) { struct stat st; char *path; - int fd, len; + int fd; + size_t len; + ssize_t read_result; if (!is_directory(git_path("worktrees/%s", id))) { strbuf_addf(reason, _("Removing worktrees/%s: not a valid directory"), id); @@ -56,10 +58,26 @@ static int prune_worktree(const char *id, struct strbuf *reason) id, strerror(errno)); return 1; } - len = st.st_size; + len = xsize_t(st.st_size); path = xmallocz(len); - read_in_full(fd, path, len); + + read_result = read_in_full(fd, path, len); + if (read_result < 0) { + strbuf_addf(reason, _("Removing worktrees/%s: unable to read gitdir file (%s)"), + id, strerror(errno)); + close(fd); + free(path); + return 1; + } close(fd); + + if (read_result != len) { + strbuf_addf(reason, + _("Removing worktrees/%s: short read (expected %"PRIuMAX" bytes, read %"PRIuMAX")"), + id, (uintmax_t)len, (uintmax_t)read_result); + free(path); + return 1; + } while (len && (path[len - 1] == '\n' || path[len - 1] == '\r')) len--; if (!len) { diff --git a/bulk-checkin.c b/bulk-checkin.c index 9a1f6c49ab..3310fd210a 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -115,7 +115,10 @@ static int stream_to_pack(struct bulk_checkin_state *state, if (size && !s.avail_in) { ssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf); - if (read_in_full(fd, ibuf, rsize) != rsize) + ssize_t read_result = read_in_full(fd, ibuf, rsize); + if (read_result < 0) + die_errno("failed to read from '%s'", path); + if (read_result != rsize) die("failed to read %d bytes from '%s'", (int)rsize, path); offset += rsize; diff --git a/csum-file.c b/csum-file.c index a172199e44..2adae04073 100644 --- a/csum-file.c +++ b/csum-file.c @@ -19,7 +19,7 @@ static void flush(struct sha1file *f, const void *buf, unsigned int count) if (ret < 0) die_errno("%s: sha1 file read error", f->name); - if (ret < count) + if (ret != count) die("%s: sha1 file truncated", f->name); if (memcmp(buf, check_buffer, count)) die("sha1 file '%s' validation error", f->name); diff --git a/notes-merge.c b/notes-merge.c index 597d43f65c..4352c34a6e 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -308,8 +308,6 @@ static void write_buf_to_worktree(const struct object_id *obj, if (errno == EPIPE) break; die_errno("notes-merge"); - } else if (!ret) { - die("notes-merge: disk full?"); } size -= ret; buf += ret; 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/packfile.c b/packfile.c index f69a5c8d60..eab7542487 100644 --- a/packfile.c +++ b/packfile.c @@ -442,6 +442,7 @@ static int open_packed_git_1(struct packed_git *p) unsigned char sha1[20]; unsigned char *idx_sha1; long fd_flag; + ssize_t read_result; if (!p->index_data && open_pack_index(p)) return error("packfile %s index unavailable", p->pack_name); @@ -483,7 +484,10 @@ static int open_packed_git_1(struct packed_git *p) return error("cannot set FD_CLOEXEC"); /* Verify we recognize this pack file format. */ - if (read_in_full(p->pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr)) + read_result = read_in_full(p->pack_fd, &hdr, sizeof(hdr)); + if (read_result < 0) + return error_errno("error reading from %s", p->pack_name); + if (read_result != sizeof(hdr)) return error("file %s is far too short to be a packfile", p->pack_name); if (hdr.hdr_signature != htonl(PACK_SIGNATURE)) return error("file %s is not a GIT packfile", p->pack_name); @@ -500,7 +504,10 @@ static int open_packed_git_1(struct packed_git *p) p->num_objects); if (lseek(p->pack_fd, p->pack_size - sizeof(sha1), SEEK_SET) == -1) return error("end of packfile %s is unavailable", p->pack_name); - if (read_in_full(p->pack_fd, sha1, sizeof(sha1)) != sizeof(sha1)) + read_result = read_in_full(p->pack_fd, sha1, sizeof(sha1)); + if (read_result < 0) + return error_errno("error reading from %s", p->pack_name); + if (read_result != sizeof(sha1)) return error("packfile %s signature is unavailable", p->pack_name); idx_sha1 = ((unsigned char *)p->index_data) + p->index_size - 40; if (hashcmp(sha1, idx_sha1)) diff --git a/pkt-line.c b/pkt-line.c index 647bbd3bce..93ea311443 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -258,7 +258,7 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size, } /* And complain if we didn't get enough bytes to satisfy the read. */ - if (ret < size) { + if (ret != size) { if (options & PACKET_READ_GENTLE_ON_EOF) return -1; diff --git a/refs/files-backend.c b/refs/files-backend.c index 38d16a13a8..4b46cd2e26 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3035,7 +3035,7 @@ static int files_reflog_expire(struct ref_store *ref_store, } else if (update && (write_in_full(get_lock_file_fd(&lock->lk), oid_to_hex(&cb.last_kept_oid), GIT_SHA1_HEXSZ) < 0 || - write_str_in_full(get_lock_file_fd(&lock->lk), "\n") < 1 || + write_str_in_full(get_lock_file_fd(&lock->lk), "\n") < 0 || close_ref_gently(lock) < 0)) { status |= error("couldn't write %s", get_lock_file_path(&lock->lk)); diff --git a/sha1_file.c b/sha1_file.c index 5a2014811f..09ad64ce55 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1748,10 +1748,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); |