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

diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 6d9a79f9b39834d953685b2641354915df3cfcbe..2706fcfaf2261e2ac2eaff5a054d0dd7a9291c0b 100644 (file)
--- 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 de26849f5551a12aac16e4121c7024ebb40502d2..7b9307aa588a70a47de4c53a380e165f90924ab9 100644 (file)
--- 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 9a1f6c49aba4ddd11284723fe781d7fbaa419f95..3310fd210a151545076169b45f5555b52acbbf9d 100644 (file)
--- 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 a172199e44bf370cd66ee7073f5a2228fd9679ef..2adae04073816a781d01d85433d8d8922baafd7f 100644 (file)
--- 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 597d43f65c664c04dc3045c3d913146321f9e97b..4352c34a6ee50f907f2d03e7d0ed1a596345a5d3 100644 (file)
--- 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 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/packfile.c b/packfile.c
index f69a5c8d607af191fa8ba04e84da8f02c40823ef..eab7542487a194ecd25708f906e66c40e1bed336 100644 (file)
--- 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 647bbd3bceda71f15fdf137a37f3fa53e6fa6d86..93ea311443a37b81b85a72dc569c715417d7ef29 100644 (file)
--- 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 38d16a13a88ac99c5c8d1aeffbe85a74ab3ef345..4b46cd2e26dbcb782d58bcf4a7bc0c0494b715a4 100644 (file)
--- 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 5a2014811fd0a42aa74b36bf3a470e471968be6f..09ad64ce555e0b908ce0d9f22219026a2d37ae0c 100644 (file)
--- 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);